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
Add:
get_all_cases
extended to support filtering and use other modules asparametrization_target
#260Add:
get_all_cases
extended to support filtering and use other modules asparametrization_target
#260Changes from 2 commits
b387ac6
83e442e
b04ab00
73a29e8
142f06a
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.
Could this be merged with the above branch of the if ?
Also, if
parametrization_target
is not used at all below this point and onlycaller_module_name
/parent_pkg_name
are, we could imagine to accept bothFinally, when None is passed, we could rely on the
get_caller_module
utility function in order to still grab the calling module (from .fixture__creation import get_caller_module
). This should only be done if actually needed (son, if a relative path or AUTO mode is used)Does it make sense ?
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.
Yes it could be merged, just made into an
else
statement. I was thinking to put it into the for loop but it made more sense to catch the error early to me.I'm not sure I understand the bullet points but given the last statement of using
get_caller_module
, how does this behaviour soundWith respect to:
Would this functionality be solved with the
cases='.'
orcases=AUTO
way of calling as illustrated in the last section of the code snippet above? If not, could you provide a code snippet of what args you would pass in and what the behaviour would be?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.
Would use
get_caller_module
so these args work as expected: YESWe would alternately accept an explicit module object (or name ?) to explicitly help resolve the caller module
I am just thinking that if we only need a module to perform this action, then we should not need users to pass a dummy function. So either the module is implicit (from the call stack,
get_caller_module
), or it is explicit and therefore we would accept thatparametrization_target
(or a new param) receives it.Maybe this can be done in a separate PR if it sounds too cumbersome. For now you can stick to the use case you had in mind (implicit or none).