Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subclass that overrides __new__ should construct instances having revealed type of subclass (not superclass) #2510

Closed
mosbasik opened this issue Oct 28, 2021 · 4 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@mosbasik
Copy link

mosbasik commented Oct 28, 2021

Description
When a subclass's __new__ returns the result of super().__new__, the revealed type of an object returned by the subclass' constructor is that of the superclass. I expect the revealed type to be that of the subclass.

Example
At my workplace, we subclass collection.namedtuple in order to define some defaults handling, arg validation and behavior associated with an immutable structure (we'd probably use a different approach today, sure; this was originally written using Python 2.7). Here's a demonstrative snippet:

from datetime import date
from typing import Optional
import collections

HolidayBase = collections.namedtuple('HolidayBase', 'name day')

class Holiday(HolidayBase):

    def __new__(cls, name: str, day: Optional[date]=None):
        '''Override to use today's date when the `day` kwarg is omitted'''
        day = day or date.today()
        obj = super().__new__(cls, name, day)
        reveal_type(obj)  # Type of "obj" is "~HolidayBase" (Sure, that makes sense)
        obj.__slots__ = ()
        return obj

    def __str__(self):
        '''Friendlier string representation'''
        return f'{self.name} ({self.day.isoformat()})'

p = Holiday('Jellybean Day')  # No `day` argument
assert str(p) == f'Jellybean Day ({date.today().isoformat()})'  # Definitely behaves like an instance of Holiday
assert isinstance(p, Holiday)  # Python thinks it's an instance of Holiday
reveal_type(p)  # Type of "p" is "~HolidayBase" (Wait, what?)

VS Code extension or command-line
I'm running Pylance v2021.10.3, which according to their CHANGELOG.md includes Pyright 1.1.182.

Additional context

Since the main reason to override __new__ is to do custom things with immutable objects, there's a decent amount of discussion online about namedtuples specifically - but they seem to fall into two buckets: discussions that predate type annotations that do talk about overriding __new__ (with examples using collections.namedtuple that look very like mine) but say nothing about types, and discussions that postdate type annotations but that only mention typing.NamedTuple, which (since that's a very different animal) don't usually apply very much to my situation.

If this isn't actually a bug and I'm holding it wrong, I would appreciate any tips about what I could do differently to address this kind of error, which is currently happening all over the place in my codebase:

holidays: list[Holiday] = [Holiday(name) for name in ['Red Day', 'Blue Day', 'White Day']]
# Expression of type "list[~HolidayBase]" cannot be assigned to declared type "list[Holiday]"
@erictraut
Copy link
Collaborator

erictraut commented Oct 28, 2021

This is a bug. This should type check without a problem. Thanks for bringing it to our attention.

@erictraut erictraut added the bug Something isn't working label Oct 28, 2021
@mosbasik
Copy link
Author

ah, just noticed you responded. Don't know if it helps, but I'd just found these two somewhat related issues, both of which have examples where a __new__ function just immediately returns a string instead of an instance of the enclosing class:

  1. __new__ Class Method And Class Call Return Types #625 closed in April 2020 as "as designed" (with a "why would one ever want to do something like that?" 😄 )
  2. Pylance didn't check return type from __new__ pylance-release#1092 resulted in a bug fix merged in March 2021 (with a "doing that is unusual, but a type checker should handle it properly")

Might help jog any memories of previous decisions made in this area!

@erictraut
Copy link
Collaborator

This will be fixed in the next release.

The super().__new__(cls) expression requires some tricky special-case handling in the type evaluator logic, and there was a bug in one of the code paths.

In your example above, you can see where this went wrong. The reveal_type(obj) call in your code sample should have revealed ~Holiday, not ~HolidayBase. If you're interested in what the ~ means here, refer to this documentation.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Oct 29, 2021
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.183, which I just published. It will also be included in the next release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants