Skip to content

ENH: set __module__ on IndexSlice #60269

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

ENH: set __module__ on IndexSlice #60269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f9604b
IndexSliceupdate
shengjie2013 Nov 9, 2024
fdb9f5c
add set_module to pandas.util._decorators
shengjie2013 Nov 9, 2024
269289c
define instance at top level
simonjayhawkins Nov 12, 2024
c6b5dbc
Merge branch 'main' into IndexSliceupdate
simonjayhawkins Nov 12, 2024
b4ca522
lint
simonjayhawkins Nov 12, 2024
84c8258
Merge branch 'IndexSliceupdate' of github.com:shengjie2013/pandas int…
simonjayhawkins Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pandas/core/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
)
from pandas.core.indexes.period import period_range
from pandas.core.indexes.timedeltas import timedelta_range
from pandas.core.indexing import IndexSlice
from pandas.core.indexing import IndexSlice as _IndexSlice
from pandas.core.series import Series
from pandas.core.tools.datetimes import to_datetime
from pandas.core.tools.numeric import to_numeric
Expand All @@ -79,6 +79,8 @@
# DataFrame needs to be imported after NamedAgg to avoid a circular import
from pandas.core.frame import DataFrame # isort:skip

IndexSlice = _IndexSlice()

__all__ = [
"array",
"ArrowDtype",
Expand Down
11 changes: 6 additions & 5 deletions pandas/core/indexing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
LossySetitemError,
)
from pandas.errors.cow import _chained_assignment_msg
from pandas.util._decorators import doc
from pandas.util._decorators import (
doc,
set_module,
)

from pandas.core.dtypes.cast import (
can_hold_element,
Expand Down Expand Up @@ -98,7 +101,8 @@


# the public IndexSlicerMaker
class _IndexSlice:
@set_module("pandas")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this....

>>> pandas.IndexSlice
<pandas._IndexSlice object at 0x7ff1e9305bd0>
>>> pandas._IndexSlice
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'pandas' has no attribute '_IndexSlice'. Did you mean: 'IndexSlice'?
>>> ```

since the purpose of #55178 is that the repr should be the path end users access the objects. This is an odd case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual IndexSlice object is defined a bit lower as IndexSlice = _IndexSlice(), so we would need to update the __module__ of that object, I think

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

pandas/pandas/core/indexing.py

Line 148 in d3c595e

IndexSlice = _IndexSlice()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, for a class instance, that doesn't actually seem to work:

In [3]: pd.IndexSlice.__module__ = "pandas"

In [4]: pd.IndexSlice
Out[4]: <pandas.core.indexing._IndexSlice at 0x7fa7d01c3610>

In this case we might need to add a def __repr__ to the _IndexSlice class if we want to improve this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we would need to update the __module__ of that object, I think

so it looks like the way this would be done is instance.__class__.__module__ = 'new_module_name' which changes the module attribute for the class which is what has been done anyway.

the output for a class is of the form <class 'pandas.core.indexing._IndexSlice'>

whereas the output for a class instance is of the form <pandas.core.indexing._IndexSlice object at 0x7fda1576c280>

now changing the __module__ attribute on the class (as done already in this PR) gives <pandas._IndexSlice object at 0x7f6442deff70> which is not correct since `_IndexSlice is not a top level class.

I'm assuming that if we want to change this, we would want <pandas.IndexSlice object at 0x7f6442deff70>? This would be technically incorrect also?

In this case we might need to add a def repr to the _IndexSlice class if we want to improve this.

in another discussion it was mentioned that the repr is not being used in IPython?

It appears that changing the __name__ attribute on the _IndexSlice class (or alias) doesn't get the desired result either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in another discussion it was mentioned that the repr is not being used in IPython?

I think that was about function objects, but here we have an instance of a class, and that should use its repr (that's how we define repr of other pandas objects)

But it seems you have found another solution anyway

class IndexSlice:
"""
Create an object to more easily perform multi-index slicing.

Expand Down Expand Up @@ -145,9 +149,6 @@ def __getitem__(self, arg):
return arg


IndexSlice = _IndexSlice()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we should keep a _IndexSlice = IndexSlice here for backwards compatibility (I know this is private, but you never know ..)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick github search I don't find much public use, but for example the stubs use the current import to get the class for type annotations

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem. can do that if you are happy with the rest of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick github search I don't find much public use, but for example the stubs use the current import to get the class for type annotations

hmm. I'll look in more detail (probs tomorrow now) before making the changes since if the class is needed for type annotations then perhaps the class should be exposed in pandas.api.typing and the repr should point to that.



class IndexingMixin:
"""
Mixin for adding .loc/.iloc/.at/.iat to Dataframes and Series.
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/api/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ def test_set_module():
assert pd.Period.__module__ == "pandas"
assert pd.Timestamp.__module__ == "pandas"
assert pd.Timedelta.__module__ == "pandas"
assert pd.IndexSlice.__module__ == "pandas"
assert pd.isna.__module__ == "pandas"
assert pd.notna.__module__ == "pandas"
assert pd.merge.__module__ == "pandas"
Expand Down