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.

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

[Doc] parametrize_with_cases takes list of such elements #257

Closed
eddiebergman opened this issue Feb 17, 2022 · 5 comments
Closed

[Doc] parametrize_with_cases takes list of such elements #257

eddiebergman opened this issue Feb 17, 2022 · 5 comments

Comments

@eddiebergman
Copy link
Contributor

eddiebergman commented Feb 17, 2022

Ello,

While the documentation, the type signature for cases doesn't seem to indicate it takes lists
of such items Callable, Type and ModuleRef while the description below says it does.

@parametrize_with_cases(argnames: str,
                        cases: Union[Callable, Type, ModuleRef] = AUTO,
                        prefix: str = 'case_',
                        glob: str = None,
                        has_tag: Union[str, Iterable[str]] = None,
                        filter: Callable = None,
                        ids: Union[Callable, Iterable[str]] = None,
                        idstyle: Union[str, Callable] = None,
                        scope: str = "function",
                        import_fixtures: bool = False
                        )

Finally, the cases argument also accepts an explicit case function, cases-containing class, module or module name; or a list of such elements. Note that both absolute and relative module names are suported.

typo in "suported" that can be fixed :)

I understand this would complicate the type signature but I was hoping to distuinguish between the two cases. The code seems to indicate the second option here.

# cases must be a list of same type
cases: Union[Callable, Type, ModuleRef, List[Callable], List[Type], List[ModuleRef]]
# or cases can be a list of any type
cases: Union[Callable, Type, ModuleRef, List[Union[Callable, Type, ModuleRef]]]

In any case, do you think it makes sense to make this more explicit? I imagine making the type signature more explicit in the docs is much too long but at least in the documentation to clarify it?

@smarie
Copy link
Owner

smarie commented Feb 21, 2022

Good catch. Maybe

CaseType = Union[Callable, Type, ModuleRef]


def parametrize_with_cases(...
                           cases=AUTO,              # type: Union[CaseType, Iterable[CaseType]]

? (plus the API reference documentation pages you mention, as they are not auto-generated :( )

Note that it should also change in get_all_cases.

If you feel like proposing a PR I would happily review it ;)

Note that we don't have typing tests in the test suite, so please check the mod with your favorite tool just to be sure

thanks in advance !

@eddiebergman
Copy link
Contributor Author

I can look into this later this week, I assume some tests to actually verify that this behaviour is correct might be required i.e. That a CaseType is possible or a list of any possible Iterable[CaseType] is possible?

I havn't looked through the tests of pytest_cases so it may already be there

@smarie
Copy link
Owner

smarie commented Feb 21, 2022

Yes the tests already support it since the corresponding code is covered https://smarie.github.io/python-pytest-cases/reports/coverage/d_e0f0fa8c1f5d62a2_case_parametrizer_new_py.html#t240 (line 245)

This is really just type hints, sorry for this :'(

@eddiebergman
Copy link
Contributor Author

Cool, thanks for pointing me to it, it should be a quick doc update then :) I'll have a look later today so!

@smarie
Copy link
Owner

smarie commented Mar 21, 2022

Closed by #259

@smarie smarie closed this as completed Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants