TYP: selection and groups type-hinting in groupby #36643
Conversation
pandas/core/groupby/grouper.py
Outdated
| @@ -600,7 +600,7 @@ def _make_codes(self) -> None: | |||
| self._group_index = uniques | |||
|
|
|||
| @cache_readonly | |||
| def groups(self) -> Dict[Hashable, np.ndarray]: | |||
| def groups(self) -> PrettyDict[Hashable, np.ndarray]: | |||
There was a problem hiding this comment.
I think this should just be Dict - while literally correct we type our API with as permissive of a type as possible for arguments, so I think should do the same on return values as well
There was a problem hiding this comment.
Agreed on arguments, especially for user-facing methods. But does that also apply to a return, where the result is known to be a PrettyDict? In general I try to be as specific with types as possible; the more specific one can be the more issues mypy can check. I also find the specificity helps with readability.
There was a problem hiding this comment.
The problem arises when users start using the return type in their own type checking, particularly when things are invariant. For all practical purposes (outside of the repr) this returns a Dict, so I think better downstream to label as such
There was a problem hiding this comment.
Thanks for the explanation, makes sense. To add to your reasons, I think PrettyDict is also not a public part of the public API. With this, it also shouldn't be leaked out via type-hints.
|
can you rebase and ping on green. |
…sc_groupby_typing � Conflicts: � pandas/core/base.py
…sc_groupby_typing
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@jreback green |
* TYP: selection and groups type-hinting in groupby * PrettyDict -> Dict * Cleanup from merge
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff