Open TomNicholas opened 10 months ago
I agree — and given there is a clear alternative to ds.dims
— ds.sizes
, it would be reasonable to put a FutureWarning
on it and then change it in a few versions.
(I thought there might be an issue for this but couldn't find one...)
This is actually more than 5 minutes of effort: once I add the FutureWarning
, then a huge number of warnings will be raised by internals. Some of those warnings then break tests that assert_no_warnings
.
To remove these warnings I have to find every location where .dims was called but a dict was intended (but not places where .dims was called and only the keys used). I can't simply grep for that...
Also relevant previous issues (very old): #921 and #1076
Also if I warn on ds.dims, then even for future-proof uses of ds.dims (e.g
for d in ds.dims
) the only way for the user to remove the warning is to
use ds.sizes.keys()
, but that's not actually what we want them to do
eventually!
On Thu, Nov 30, 2023, 5:47 PM Maximilian Roos @.***> wrote:
I agree — and given there is a clear alternative to ds.dims, it would be reasonable to put a FutureWarning on it and then change it in a few versions.
(I thought there might be an issue for this but couldn't find one...)
— Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/issues/8496#issuecomment-1834680374, or unsubscribe https://github.com/notifications/unsubscribe-auth/AISNPI5TSOV72JSNVHOFYQLYHEEJFAVCNFSM6AAAAABAB43R26VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZUGY4DAMZXGQ . You are receiving this because you authored the thread.Message ID: @.***>
Yeah that struck me too. It seems like a custom Mapping object would be the way to do. Raise warnings on __getitem__
but not on __iter__
.
Oh that's clever! I did not think of that.
then a huge number of warnings will be raised by internals.
One insight I've learned form Stephan on this repo is to treat warnings in the test suite as a metric for the impact on users. This one seems pretty big! =)
This one seems pretty big! =)
I hope that's a good thing 😅
@dcherian I think that ds.groupby.dims
should also be changed for consistency too FYI.
Are we ready to complete this deprecation cycle? I ask because it would be nice to release DataTree
with DataTree.dims
able to be start with the new intended behaviour...
What is your issue?
This is inconsistent:
Surely
ds.dims
should return something like aFrozenset({'x', 'y'})
? (because dimension order is meaningless when you have multiple arrays underneath - see https://github.com/pydata/xarray/issues/8498)