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

TYP: FrameOrSeriesUnion #36475

Closed
rhshadrach opened this issue Sep 19, 2020 · 6 comments
Closed

TYP: FrameOrSeriesUnion #36475

rhshadrach opened this issue Sep 19, 2020 · 6 comments
Labels
Needs Discussion Requires discussion from core team before further action Typing type annotations, mypy/pyright type checking

Comments

@rhshadrach
Copy link
Member

From the discussion in PR #36409 it was mentioned that it is undesirable to type API functions as NDFrame as this may be confusing from a user perspective since it shows up in the docs. Currently the difference between NDFrame and FrameOrSeriesUnion = Union[DataFrame, Series] is that the former can be used in methods of NDFrame (and those called from NDFrame methods) whereas the latter cannot.

Is there a benefit to this restriction? If not, I propose changing FrameOrSeriesUnion to be an alias of NDFrame. This would then be user-friendly for the docs.

cc @simonjayhawkins @WillAyd @jreback

@rhshadrach rhshadrach added Needs Discussion Requires discussion from core team before further action Typing type annotations, mypy/pyright type checking labels Sep 19, 2020
@simonjayhawkins
Copy link
Member

This would then be user-friendly for the docs.

at the moment, sphinx expands the aliases rather than showing the alias. see return type of https://pandas.pydata.org/pandas-docs/dev/reference/api/pandas.concat.html

see also #33025 and #28585

@simonjayhawkins
Copy link
Member

#29480 sort of related as well.

@rhshadrach
Copy link
Member Author

Thanks for the background @simonjayhawkins. I see a different result here:

https://pandas.pydata.org/docs/reference/api/pandas.concat.html

where the type-hints are not expanded. Is this a recent change that will impact the 1.2 docs when released?

@simonjayhawkins
Copy link
Member

tbh, not even sure why they are appearing for pd.concat in html docs as they as supposed to be disabled, see #33312

for normal python help...

>>> import pandas as pd
>>> help(pd.concat)
Help on function concat in module pandas.core.reshape.concat:

concat(objs: Union[Iterable[ForwardRef('NDFrame')], Mapping[Union[Hashable, NoneType], ForwardRef('NDFrame')]], axis=0, join='outer', ignore_index: bool = False, keys=No
ne, levels=None, names=None, verify_integrity: bool = False, sort: bool = False, copy: bool = True) -> Union[ForwardRef('DataFrame'), ForwardRef('Series')]

it even includes the ForwardRef! (this can now be removed, but not for backports, see #36034)

@rhshadrach
Copy link
Member Author

@simonjayhawkins This is an issue with Sphinx, currently open:

sphinx-doc/sphinx#7785

@simonjayhawkins
Copy link
Member

From the discussion in PR #36409 it was mentioned that it is undesirable to type API functions as NDFrame as this may be confusing from a user perspective since it shows up in the docs.

type annotations are no longer displayed in the docs

If not, I propose changing FrameOrSeriesUnion to be an alias of NDFrame

no longer relevant. #41955

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion Requires discussion from core team before further action Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

No branches or pull requests

2 participants