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

Fix generic inheritance for dataclass init methods #9380

Merged
merged 3 commits into from Nov 28, 2020

Conversation

natemcmaster
Copy link
Contributor

Description

Fixes #7520

Updates the dataclasses plugin. Instead of directly copying attribute type along the MRO, this first resolves typevar in the context of the subtype.

Test Plan

Added 4 new test cases to cover various generic inheritance scenarios.

@natemcmaster
Copy link
Contributor Author

natemcmaster commented Aug 30, 2020

One of the test jobs failed (python3.5.1 with mypyc). The errors appear unrelated to my changes, and that same job passed when I ran the same commit hash - https://travis-ci.org/github/natemcmaster/mypy/builds/722519864. Is this failure due to flaky tests?

@JelleZijlstra
Copy link
Member

@natemcmaster it's likely due to a recent setuptools release that broke stuff; I submitted a PR that will hopefully fix CI.

@natemcmaster
Copy link
Contributor Author

@JelleZijlstra thanks! I merged latest updates from master into my PR. CI is passing now on this PR.

@natemcmaster
Copy link
Contributor Author

Friendly bump for review :)

@natemcmaster
Copy link
Contributor Author

Hello there, checking in again. I merged from master and pushed to my branch to ensure this PR is still merge-conflict free and passing tests. Any chance I can get a review? Thanks!

@nathan-chappell
Copy link

nathan-chappell commented Nov 20, 2020

Any status on the review? I have a super lame generic dataclass inheritance chain mess that's waiting on it. And I'm sure you don't have anything better to do...

@natemcmaster
Copy link
Contributor Author

I haven't heard anything from the repo maintainers :-/

@hauntsaninja
Copy link
Collaborator

Thanks for this (and #9383)!

Sorry for the delay here, mypy's just been really short on reviewer bandwidth / I've shied away from reviewing these because I'm not familiar with these plugins. But the change make sense, your tests look good and mypy_primer doesn't surface any complaints.

@hauntsaninja hauntsaninja merged commit 98beb8e into python:master Nov 28, 2020
@natemcmaster
Copy link
Contributor Author

Thanks for merging this and the other PR, @hauntsaninja! I totally understand the delay. I used to be a maintainer on a big open source project myself :). Thanks for following up on this!

@natemcmaster natemcmaster deleted the dataclass-generic branch November 28, 2020 02:14
@binishkaspar
Copy link

@natemcmaster Fix works very well. However the following code raises INTERNAL ERROR,

DRIVE_TYPE = TypeVar('DRIVE_TYPE', bound='Driver')

# This WORKS
# DRIVER_STATE_TYPE = TypeVar('DRIVER_STATE_TYPE', bound='DriverState')


@dataclass
class DriverState(Generic[DRIVE_TYPE]):
    driver: DRIVE_TYPE
    pid: str = ''


# This doesn't work
DRIVER_STATE_TYPE = TypeVar('DRIVER_STATE_TYPE', bound='DriverState')


@dataclass
class Driver(Generic[DRIVER_STATE_TYPE]):
    config: DRIVER_STATE_TYPE


@dataclass
class ESState(DriverState['ESDriver']):
    special_name: str = 'ES Driver'


@dataclass
class ESDriver(Driver[
    ESState
]):
    def name(self) -> None:
        print(self.config.special_name)

Is it because supertype not recursively expanded?

@natemcmaster
Copy link
Contributor Author

@binishkaspar can you open a new issue? I'm not sure of the expected behavior of TypeVar with an open generic bound.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dataclass does not properly support generics
5 participants