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.

Sign up for GitHub

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

Jump to bottom

Fix the MRO order and correctly add metaclass types #88

Merged
merged 2 commits into from
Feb 9, 2023
Merged

Fix the MRO order and correctly add metaclass types #88

merged 2 commits into from
Feb 9, 2023

Conversation

euresti
Copy link
Contributor

@euresti euresti commented Feb 8, 2023 •
edited by kedder
Loading

This fixes the caching error at #86 by ensuring that the faketi has the same metaclass_type as the TypeInfo that is being faked. I also adjusted the MRO so that builtins.object is correctly ignored by the mypy code. (It expected the MRO to end with builtins.object)

Note: This does not fix the fact that mypy 1.0.0 will show a:

error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]

This only ensures that error shows up every time so that it can be # type: ignored if you want.

Closes #86

kedder
kedder reviewed Feb 8, 2023
View reviewed changes
src/mypy_zope/plugin.py
impl.mro.append(faketi)
faketi.metaclass_type = iface.metaclass_type
# Insert the TypeInfo before the builtins.object that's at the end.
impl.mro.insert(len(impl.mro) - 1, faketi)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure builtins.object is the last item each and every time? Would it make sense to add some kind of assertion on that in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure since mypy will not let you put a base class of object first:
See: https://mypy-play.net/?mypy=0.981&python=3.11&gist=2ba86753872fd1a1fd4fffad9c4ded9a

I'm happy to add an assert too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, if you can add the assert, I think we can merge this. Thanks for digging into it!

@DMRobertson DMRobertson mentioned this pull request Feb 9, 2023
Merged
@euresti euresti requested a review from kedder February 9, 2023 18:50
@euresti
Copy link
Contributor Author

euresti commented Feb 9, 2023

Assertion added. Thanks!

kedder
kedder approved these changes Feb 9, 2023
View reviewed changes
Copy link
Member

@kedder kedder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! 👍

@kedder kedder merged commit ec6800c into Shoobx:master Feb 9, 2023
@kedder
Copy link
Member

kedder commented Feb 9, 2023 •
edited
Loading

Ok, this assertion caused a meltdown on master when combined with #89... What a coincidence!

@kedder
Copy link
Member

kedder commented Feb 9, 2023

actually, it is not that assertion, disregard...

kedder added a commit that referenced this pull request Feb 9, 2023
@kedder
Fix metaclass conflict
858bf45 
This fixes the regression when #88 combined with #89: mypy-1.0 started
producing the error:

```
Metaclass conflict: the metaclass of a derived class must be a
(non-strict) subclass of the metaclasses of all its bases
```
@kedder kedder mentioned this pull request Feb 9, 2023
Merged
@euresti euresti deleted the fix_order_and_metaclass branch February 9, 2023 21:03
kedder added a commit that referenced this pull request Feb 9, 2023
@kedder
Fix metaclass conflict
d06e2ac 
This fixes the regression when #88 combined with #89: mypy-1.0 started
producing the error:

```
Metaclass conflict: the metaclass of a derived class must be a
(non-strict) subclass of the metaclasses of all its bases
```
kedder added a commit that referenced this pull request Feb 10, 2023
@kedder
Fix metaclass conflict
a9954b7 
This fixes the regression when #88 combined with #89: mypy-1.0 started
producing the error:

```
Metaclass conflict: the metaclass of a derived class must be a
(non-strict) subclass of the metaclasses of all its bases
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kedder kedder kedder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Caching error (Metaclass conflict) with mypy 0.991
2 participants
@euresti @kedder