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
    @rytilahti what is in the __init__.py? Also how are you calling mypy?
    Teemu R.
    @rytilahti
    oh, sorry, the link didn't got pasted... here's the init: https://github.com/rytilahti/python-mirobo/blob/master/mirobo/__init__.py
    and I'm calling mypy with 'mypy mirobo'
    Teemu R.
    @rytilahti
    hm. so the problem is actually that those errors are coming from the using class, where 'from . import VacuumStatus' is used for example
    so a cyclical dep :)
    Michael Lee
    @Michael0x2a

    In Python 3.6+, there are two new ways of annotating fields for complex objects: using class attributes, like so:

    class Foo:
        field1: int
        field2: List[str]
    
        def __init__(self, arg: Bar) -> None:
            # Complex setup logic here

    ...or by annotating instance variables inside __init__, like so:

    class Foo:
        def __init__(self, arg: Bar) -> None:
            self.field1: int
            self.field2: List[str]
    
            # Complex setup logic here

    Is there any sort of official ruling on which version is considered more Pythonic? I know I prefer the second version because it seems conceptually cleaner, but the first version does look more aesthetically pleasing.

    Ethan Smith
    @ethanhs
    @Michael0x2a These are different things.Consider:
    >>> class A:
    ...     attr = 1
    ...
    >>> a = A()
    >>> a.attr
    1
    >>> A.attr = 5
    >>> a.attr
    5
    >>> class B:
    ...     def __init__(self):
    ...         self.attr = 1
    ...
    >>> b = B()
    >>> b.attr
    1
    >>> B.attr = 5
    >>> b.attr
    1 
    But in general, I find class variables much more Pythonic, as they are in the definition of the class, which is where I think most people look for attributes.
    Michael Lee
    @Michael0x2a
    I'm fully aware of that. This is in reference to PEP 526 -- see https://www.python.org/dev/peps/pep-0526/#class-and-instance-variable-annotations
    Ethan Smith
    @ethanhs
    they are less explicit if they are are instance attributes
    Michael Lee
    @Michael0x2a
    (By "conceptually cleaner", I was referring to the exact thing you pointed out)
    Ethan Smith
    @ethanhs
    ah, I see. there is a section on Function Annotations in PEP 8, but perhaps a section on class variables related to PEP 526 would be useful.
    Michael Lee
    @Michael0x2a
    Once a ruling is established, probably, yeah. (The section of PEP 526 I linked you to actually includes both variants, but spends a lot of time covering the first variation but concludes by implying the second variation is the convention, which is a little ambiguous imo)
    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__