CLN: De-privatize core.common funcs, remove unused by jbrockmendel · Pull Request #22001 · pandas-dev/pandas (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Moves a few console-checking functions to io.formats.console.
A bunch of core.common functions were never used outside of tests, got rid of em.
The ones I left alone were _any_not_none, _all_not_none etc, as I'm inclined to think these should be removed in favor of python builtins.
| def _apply_if_callable(maybe_callable, obj, **kwargs): |
| def apply_if_callable(maybe_callable, obj, **kwargs): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prob should move all of the indexing ones to a new indexing module
maybe pandas.core.indexing.utils
(and obviously move indexing.py itself)
| returns True if running under python/ipython interactive shell |
| """ |
| from pandas import get_option |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already imported
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a different function, not at module level
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh ok
[CLN] De-privatize core.common funcs, remove unused CLN: De-privatize core.common funcs, remove unused
| # ---------------------------------------------------------------------- |
|---|
| # Detect our environment |
| def in_interactive_session(): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT the 2nd 2 routines are not used at all? can you remove if that is the case
note pandas.core.common is technically public. We never made explicity made this non-public, so have to be careful about name changes here. I find these ok, but in the longer term we should probably change this.
note pandas.core.common is technically public. We never made explicity made this non-public, so have to be careful about name changes here.
Ah, yes, that's a good point. The ones we previously moved out we deprecated explicitly.
I don't think it would be too hard to deprecate all the renamed functions? (create the old ones calling the the new one + with added deprecation warning in a loop?)
I'm unclear on how to move forward. If core.common is public, does that mean we need to not-remove the (deprecated) environment-detecting and other unused functions? Do we need to wrap+expose the currently-private functions?
I think we only need to care about the changed non-private functions (e.g. no leading _).
Its fair game to move all of the console functions, as well as remove the unused functions. I don't think we should go crazy with deprecations. Maybe just add a blanket whatsnew. We only deprecated things because these were very commonly used function, all of the is_* ones (which moved to pandas.api.types).
I will have another look and indicate anything we actually need to deprecate.
I am actually ok with these changes as-if (just add a whatsnew note). And add a disclaimer on the doc-string that this is a non-public module.
we long ago declared the pandas.core namespace private (it would break too much to actually change this to panda._core, though maybe we should actually do that.
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request