Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
✨ feat: add Quantity API #16
✨ feat: add Quantity API #16
Changes from 6 commits
d86b919
661dbc3
c27109e
6639f20
ed4d762
83d7c28
b27ff55
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about a structure like this when I wrote
test_operations
, but I rather dislike that it requiers every method to have that explicit fixture: I thinksetup_class()
is considerably nicer: one just does tests on a given setup, which includes a given array type. Unfortunately, I could not seem to find a clean way in which one can properly parametrize classes by their setup, though I'll admit that I've never really understood the whole fixture process -- I find them rather unreadable (it seemspytest
makes up its own language within a language rather than just using standard python stuff).Anyway, long & short, I'd prefer to stick to one way of parametrizing over the array namespaces, ideally one that doesn't require to do setup in each method...
Since that is somewhat orthogonal to the goals of this PR, OK to just push back what I had and merge? (I'm definitely not stuck to my class creation code, but would like something that does not add boilerplate to each test!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively we can just make a class-scoped fixture (https://docs.pytest.org/en/7.1.x/how-to/fixtures.html#scope-sharing-fixtures-across-classes-modules-packages-or-session) by moving
def array_and_quantity(request):
into the class and adding the argument to the fixture. It neatly replacesclass_setup
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try running pytest in verbose mode, it now repeats the whole class as a block for each array library.