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

Ban defining __repr__ and __str__ in stubs #109

Merged
merged 6 commits into from
Jan 20, 2022
Merged

Ban defining __repr__ and __str__ in stubs #109

merged 6 commits into from
Jan 20, 2022

Conversation

AlexWaygood
Copy link
Collaborator

No description provided.

@AlexWaygood
Copy link
Collaborator Author

I've realised that I actually have no idea whether this is compatible with Python 2 or not.

@AlexWaygood
Copy link
Collaborator Author

I've realised that I actually have no idea whether this is compatible with Python 2 or not.

Seems like it isn't.

@JelleZijlstra
Copy link
Collaborator

That's only if you use old-style classes. If you use a new-style class (with class Foo(object):), __repr__ does exist. (I don't really know how old-style classes worked and don't much care.)

@AlexWaygood
Copy link
Collaborator Author

That's only if you use old-style classes. If you use a new-style class (with class Foo(object):), __repr__ does exist. (I don't really know how old-style classes worked and don't much care.)

Are we 100% sure that all Python 2 classes in typeshed are new-style classes?

@JelleZijlstra
Copy link
Collaborator

JelleZijlstra commented Jan 19, 2022 •
edited by AlexWaygood
Loading

Are we 100% sure that all Python 2 classes in typeshed are new-style classes?

Probably not. But it doesn't really matter anyway, removing __repr__ only affects direct calls to Foo.__repr__(). And I don't think type checkers really understand old-style classes either.

@AlexWaygood
Copy link
Collaborator Author

typeshed PR here: python/typeshed#6968

@AlexWaygood AlexWaygood reopened this Jan 19, 2022
JelleZijlstra
JelleZijlstra reviewed Jan 20, 2022
View reviewed changes
pyi.py Show resolved Hide resolved
@AlexWaygood
Copy link
Collaborator Author

AlexWaygood commented Jan 20, 2022 •
edited
Loading

Okay, I have a test case that I only want to run on 3.8+ (the code I've written is 3.7-compatible, but the test I've written for the 3.8+ case isn't). @Akuli, any thoughts on the simplest way to do that?

AlexWaygood added a commit to AlexWaygood/typeshed that referenced this pull request Jan 20, 2022
@AlexWaygood
Remove unneeded # noqa comment
00489fd 
I added this in python#6968 to make the CI go green for PyCQA/flake8-pyi#109, but we've since decided that we only want to raise errors for `__str__` and `__repr__` if the signature is an exact match for `object.__str__`/`object.__repr__`. So this `# noqa` shouldn't be necessary.
@AlexWaygood AlexWaygood mentioned this pull request Jan 20, 2022
Merged
@AlexWaygood
Copy link
Collaborator Author

(Closing and reopening to check that typeshed-primer still passes after python/typeshed#6979.)

@AlexWaygood AlexWaygood reopened this Jan 20, 2022
JelleZijlstra
JelleZijlstra reviewed Jan 20, 2022
View reviewed changes
tests/classdefs.pyi Outdated Show resolved Hide resolved
@AlexWaygood AlexWaygood mentioned this pull request Jan 20, 2022
Closed
@AlexWaygood
Copy link
Collaborator Author

I have deleted the Python 3.7-incompatible test case for now, and created #111.

@JelleZijlstra JelleZijlstra merged commit 43fea85 into PyCQA:master Jan 20, 2022
@AlexWaygood AlexWaygood deleted the repr branch January 20, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra Awaiting requested review from JelleZijlstra

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

Successfully merging this pull request may close these issues.
2 participants
@AlexWaygood @JelleZijlstra