issue_comments

In the last commit I added the xarray.indexes namespace from which we can import Index, PandasIndex and PandasMultiIndex.

Thanks everyone for the feedback and review!

I think this is ready to merge, if we agree to address the coord_names typing issue in another PR?

I personally would still choose to put indexes stuff in a separate namespace, just because it's neater, but I can see it's borderline.

On Wed, 14 Sep 2022, 06:33 Benoit Bovy, @.***> wrote:

Have you thought about whether we might want to expose a separate public xarray.indexes namespace?

Yes I've been thinking about it and I agree I find it cleaner than exposing all of this in Xarray's main namespace. There's a few (minor) cons, though:

• I think the indexes.py and indexing.py modules and their content are well located in core
• We could create a xarray/indexes/init.py and import there a few "public" classes from core, but is it worth it? I'm not sure if the number of Xarray built-in indexes will grow much beyond PandasIndex and PandasMultiIndex. Perhaps it's preferable not?
• Things like CFTimeIndex are already imported in Xarray's main namespace

— Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_pydata_xarray_pull_6971-23issuecomment-2D1246569430&d=DwMCaQ&c=009klHSCxuh5AI1vNQzSO0KGjl4nbi2Q0M1QLJX9BeE&r=qdISi9HqjazmE0DcySuXts3OlnplnLfKjH4hpzAV0xo&m=4E5eW5IsNTqFQTrWcdzS851OngwlYEdG3SG0WlL5z0sbHu692Rkq4bkhw8yxynW1&s=s8yiD2RYG-LEkCEiuSDT6KhIowl7VtGsnb_6GuYOwZk&e=, or unsubscribe https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AISNPI6DG7BJG5WWV7VBCEDV6GSXRANCNFSM6AAAAAAQBLFT4I&d=DwMCaQ&c=009klHSCxuh5AI1vNQzSO0KGjl4nbi2Q0M1QLJX9BeE&r=qdISi9HqjazmE0DcySuXts3OlnplnLfKjH4hpzAV0xo&m=4E5eW5IsNTqFQTrWcdzS851OngwlYEdG3SG0WlL5z0sbHu692Rkq4bkhw8yxynW1&s=bAX5LysTxNxkTVXx0Tv75_8-UZ5okn0yuHXvGeGScGg&e= . You are receiving this because you commented.Message ID: @.***>

Have you thought about whether we might want to expose a separate public xarray.indexes namespace?

Yes I've been thinking about it and I agree I find it cleaner than exposing all of this in Xarray's main namespace. There's a few (minor) cons, though:

• I think the indexes.py and indexing.py modules and their content are well located in core
• We could create a xarray/indexes/__init__.py and import there a few "public" classes from core, but is it worth it? I'm not sure if the number of Xarray built-in indexes will grow much beyond PandasIndex and PandasMultiIndex. Perhaps it's preferable not?
• Things like CFTimeIndex are already imported in Xarray's main namespace

In this PR I also imported the Index base class in Xarray's root namespace.

It is needed for custom indexes and it's just a little more convenient than importing it from xarray.core.indexes. Should we do the same for PandasIndex and PandasMultiIndex subclasses? Maybe if one wants to create a custom index inheriting from it. PandasMultiIndex factory methods could be also useful if we depreciate passing pd.MultiIndex objects as DataArray / Dataset coordinates.

Have you thought about whether we might want to expose a separate public xarray.indexes namespace? Then as the list of helpers for creating custom index objects grows they could live in there, so we might have xarray.Index, but xarray.indexes.PandasIndex, xarray.indexes.PandasMultiIndex, xarray.indexes.PeriodicBoundaryIndex, xarray.indexes.OtherDomainAgnosticIndex etc. all listed in the docs API page ?

Should we make the index_cls argument of set_xindex optional?

I ended up doing it. It is convenient for setting a pandas index for a non-dimension coordinate, which is currently not possible to do with set_index(). For unindexed dimension coordinates (e.g., now possible after renaming coordinates or dimensions), I find the syntax set_index(x="x") a bit weird compared to set_xindex("x").

BTW, viewing pull-request doc builds on RTD seems broken? Clicking on the "Details" link of the corresponding check leads to a 404.

Also, since .drop_indexes is new API I didn't feel the need to implement the old behavior regarding pandas multi-indexes (restored in #6592 and #6798 but deprecated anyway). @dcherian what do you think?