by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Ethan Smith
    @ethanhs
    Well, both are supported so it makes sense to have both
    Ivan Levkivskyi
    @ilevkivskyi
    We considered this question while writing PEP 526. We decided to allow both (class body and __init__). There is a short section about this in Rejected ideas.
    Michael Lee
    @Michael0x2a
    I guess I was wondering if one of the approaches was recommended over the other, from a stylistic point of view
    I know both are allowed, just not definitively which one is recommended
    Ivan Levkivskyi
    @ilevkivskyi
    I don't have preferences here. IIRC Guido prefers in class body.
    This is also the style mostly used in mypy
    (but in 3.5 version x = None # type: int)
    Michael Lee
    @Michael0x2a
    And I guess an implicit second question I have is if that style guideline ought to be formally written down somewhere (e.g. in PEP 8) or if we should leave things as is, though perhaps that's more a question for Guido
    Guido van Rossum
    @gvanrossum
    There seem to be strong opinions either way. I do indeed prefer putting attribute annotations in the class body rather than sprinkling them throughout __init__ and other methods. I also think that with PEP 526 this will be the future (also with things like class-based NamedTuple declarations and possibly https://github.com/ericvsmith/dataclasses).
    Michael Lee
    @Michael0x2a
    @gvanrossum thanks!
    Guido van Rossum
    @gvanrossum
    Everyone, I've pushed a new version of mypy_extensions to PyPI: https://pypi.python.org/pypi/mypy_extensions/0.3.0 -- please test.
    Max
    @pkch
    I noticed a perfectly correct but, I feel, a slightly unfortunate behavior of the type system:
    class Point(NamedTuple('Point', [('x', float), ('y', float)])):
        def __add__(self, other: Point) -> Point:  # E:  error: Argument 1 of "__add__" incompatible with supertype "tuple"
            return Point(self.x + other.x, self.y + other.y)
    Of course, it's correct because NamedTuple is a subclass of tuple, and (1, 2) + (3, 4) is a pre-defined operation in python. I think it's unfortunate because deriving things like Point or Vector from namedtuple is a perfect idiom in non-typed python code; so it's sad to lose that idiom in typed python.
    Max
    @pkch
    I don't see any solution to this (apart from the option of allowing programmers to derive implementation from a class X without subtyping class X, which was rejected IIRC?)
    Ivan Levkivskyi
    @ilevkivskyi

    use other: object and:

    class Point(tuple):
        def __init__(self, x):
            self.x = x
        def __add__(self, other: object) -> Point:
            if isinstance(other, Point):
                return Point(self.x + other.x)
            return NotImplemented

    this should typecheck and also will help Python behave better.

    Max
    @pkch
    :clap: I now remember I had literally the same problem a few months back. I need some memory enhancing things.
    Ivan Levkivskyi
    @ilevkivskyi
    Unfortunately, there is a bug in this pattern with named tuples: they interact strangely with isinstance (IIRC there is already an issue for this)
    Max
    @pkch
    yup python/mypy#3419
    Of course, other than the bug, there's nothing wrong with deriving from NamedTuple right? Maybe it's worth including in the tutorial at some point? It's kinda a nice pattern.
    Max
    @pkch
    Actually, there's one drawback to this pattern: the (incorrect) Point(1, 1) + [2, 2] will type check, and the error will only happen in run-time. In the original code I posted, the problem would have been identified at compile time.
    Ivan Levkivskyi
    @ilevkivskyi
    There was an idea to allow subclassing without subtyping, see python/typing#241, but it stalled.
    Max
    @pkch
    Yeah, but I guess for the time being # type: ignore next to the def __add__ is an ok'ish solution: IIUC, its only impact on the type checker would be to (effectively) disable subtype method consistency check.
    Elazar Gershuni
    @elazarg
    I'm not sure that addition of points is a good idea - technically the difference between two points is a vector, not a point
    Max
    @pkch
    Yes yes, I just realized that but didn't want to distract from the original question :) Just replace Point with Vector in my earlier examples. Vector(1,2) + Vector(1,3) is a reasonable overload.
    Max
    @pkch
    @elazarg although.. how would you calculate a center of mass of several Point if you can't sum them up?..
    Elazar Gershuni
    @elazarg
    Max
    @pkch
    And @ilevkivskyi I literally just got the very same bug that I was warning myself about: Point(1, 1) + SomethingElseInheritedFromTuple(1, 2), not caught at compile time. Now that it actually happened, I strengthened my opinion on that, and I no longer think using def __add__(self, other: object) is an acceptable solution. The only solution I personally find acceptable is to not derive from NamedTuple or tuple, and just manually implement __eq__, __hash__, __repr__, etc in my own generic class MyTuple that does not define + and derive my Vector/Point/etc. from that class. (Well, I could wrap a tuple object inside MyTuple and forward the useful methods but not the ones I don't want. So I do get some code reuse.)
    Elazar Gershuni
    @elazarg
    My earlier note was actually OT and I didn't mean to post it. Turns out that hitting the return button posts the message... Sorry!
    Max
    @pkch
    The main problem is that even if I define __add__(self, rhs: Point): # type: ignore, mypy still won't catch the cases where rhs is an unrelated tuple-derived object. Mypy correctly notices that + is defined for tuple, and I can't tell it that for Point summation, the rhs must be a Point.
    Conceptually, I think it's understandable, namedtuples are mostly used in contexts where the + meaning concatenation is reasonable. It's just an unfortunate reality for cases where + needs to be pointwise.
    Ivan Levkivskyi
    @ilevkivskyi
    What about using a different symbol? Maybe __matmul__?
    :-)
    Max
    @pkch
    :laughing: but yes, that's one way if nobody needs scalar product or something
    Ivan Levkivskyi
    @ilevkivskyi
    Also probably the right way is just wait for dataclasses PR
    sorry PEP
    then you will not need to manually implement __eq__, __hash__, __repr__, etc.
    and just add your favourite __add__
    it is also a part of the current design that dataclasses should play nicely with static type checkers.
    Max
    @pkch
    Yes, that would be awesome, I didn't know about dataclasses pep, just read it.
    Ethan Smith
    @ethanhs
    Wanted to update that I have my runtime type analyzer working to the point where it works for running over all of mypy! No more crashes :) Right now I have it serialize the type information as JSON. It isn't quite as fast as I'd hoped (running mypy on itself takes 140s as opposed to 17.5s normally), but I have yet to optimize things. Based on this tool, running mypy on itself generates 26,152 frames analyzed for just mypy files alone.
    Jukka Lehtosalo
    @JukkaL
    anybody happen to be around who can spare a few minutes to test mypy 0.520 rc on windows?
    Ethan Smith
    @ethanhs
    I can
    just a second
    Jukka Lehtosalo
    @JukkaL
    Ethan Smith
    @ethanhs
    Im running tests on git master atm, I will test the 64 bit wheel too. Do you want me to test the 32 bit one as well?
    Jukka Lehtosalo
    @JukkaL
    if you can easily test that the 32 bit one installs and you can run mypy it should be enough
    Jelle Zijlstra
    @JelleZijlstra
    should the crash from python/mypy#3687 be fixed before you release 0.520?
    it's apparently a regression since 0.511
    Michael Lee
    @Michael0x2a
    FWIW I was able to install the 32 bit one just now, though just to be safe, @ethanhs should probably confirm
    Ethan Smith
    @ethanhs
    Everything works as expected, 32 bit install works. I too am concerned about python/mypy#3687