-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add a FutureWarning to Dataset.__iter__ and Dataset.__len__ #1658
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.
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
Conversation
xref GH884 This will allow us to resolve a long-standing pain-point with consistency between pandas and xarray. CC pydata/xarray
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just few doc changes. Did you test test this by setting these new warnings to errors to make sure you've found all the locations that need to changed?
doc/data-structures.rst
Outdated
``ds.keys()``) the next major release of xarray, to only include data | ||
variables instead of both data variables and coordinates. In the meantime, | ||
please prefer iterating over keys in ``ds.data_vars`` or ``ds.coords``, or | ||
``ds.variables`` if you need both at once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could use some polishing. The first sentence should probably be split in two. Also, the "please prefer" statement is a bit awkward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to keep this as a simple version of the full deprecation warning, and leave the detailed rationale to whats-new.rst
. Let me know if this is any better...
doc/whats-new.rst
Outdated
for membership in ``DataArray.coords``. In the next major version of | ||
xarray, it will check membership in the array data found in | ||
``DataArray.values`` instead (:issue:`1267`). | ||
- Directly iteration over and counting a ``Dataset`` (e.g., ``[k for k in ds]``, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly->Direct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Yes, this was tested by making the warnings errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
This will allow us to resolve a long-standing pain-point with consistency between pandas and xarray.
git diff upstream/master | flake8 --diff
whats-new.rst
for all changes andapi.rst
for new APICC @pydata/xarray