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

Mark base node classes as ABCs #11676

Closed
bluetech opened this issue Dec 6, 2023 · 0 comments · Fixed by #11677
Closed

Mark base node classes as ABCs #11676

bluetech opened this issue Dec 6, 2023 · 0 comments · Fixed by #11677
Assignees
@bluetech
Labels
topic: collection related to the collection phase topic: typing type-annotation issue type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@bluetech
Copy link
Member

bluetech commented Dec 6, 2023

The Node class hierarchy has some classes which are only meant to be used as base classes, never instantiated directly. These are Node, Collector, Item, File, FSCollector, PyCollector.

I propose we mark them as abstract i.e. inherit from abc.ABC. Reasons:

  • Trying to instantiate fails quickly at runtime
  • Type checkers issue an error if trying to instantiate
  • The docs will show abstractmethod on the abstract methods that need to be implemented (collect or runtest), and show inheritance from ABC making it clear it's a base class

It seems somewhat hard to deprecate, and it will only cause errors when the code is already very wrong. So I propose to do it in pytest 8.0 without deprecation.
@bluetech bluetech added type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature topic: collection related to the collection phase type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog topic: typing type-annotation issue labels Dec 6, 2023
@bluetech bluetech self-assigned this Dec 6, 2023
bluetech added a commit to bluetech/pytest that referenced this issue Dec 6, 2023
@bluetech
nodes,python: mark abstract node classes as ABCs
81a1237 
Fixes pytest-dev#11676
@bluetech bluetech mentioned this issue Dec 6, 2023
Merged
bluetech added a commit to bluetech/pytest that referenced this issue Dec 7, 2023
@bluetech
nodes,python: mark abstract node classes as ABCs
06fe47d 
Fixes pytest-dev#11676
bluetech added a commit to bluetech/pytest that referenced this issue Dec 7, 2023
@bluetech
nodes,python: mark abstract node classes as ABCs
0ae02e2 
Fixes pytest-dev#11676
@str0zzapreti str0zzapreti mentioned this issue Feb 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bluetech bluetech

Labels
topic: collection related to the collection phase topic: typing type-annotation issue type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

nodes,python: mark abstract node classes as ABCs bluetech/pytest
1 participant
@bluetech