Skip to content
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

Jump to bottom

Allow opening datasets with nD dimenson coordinate variables. #7989

Merged
merged 6 commits into from
Jul 19, 2023
Merged

Allow opening datasets with nD dimenson coordinate variables. #7989

merged 6 commits into from
Jul 19, 2023

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Jul 15, 2023 •
edited
Loading

Avoid automatic creating of Index variable when nD variable shares name with one of its dimensions.

Closes #2233

url = "http://www.smast.umassd.edu:8080/thredds/dodsC/FVCOM/NECOFS/Forecasts/NECOFS_GOM3_FORECAST.nc"
ds = xr.open_dataset(url, engine="netcdf4")
display(ds)

xr.testing._assert_internal_invariants(ds, check_default_indexes=False)  ! no raise on #7368

The internal invariants assert fails on main but succeeds on #7368. EDIT: now fixed the invariants check.

image

@dcherian
Warn instead of raising for nD index variable
ec27da4 
Avoid automatic creating of Index variable
when nD variable shares name with one
of its dimensions.

Closes pydata#2233
@dcherian dcherian requested a review from benbovy July 15, 2023 17:33
@dcherian dcherian changed the title Warn instead of raising for nD index variable Warn instead of raising when automatically creating nD index variable Jul 15, 2023
@benbovy
Copy link
Member

benbovy commented Jul 16, 2023

Wow I'm positively surprised that it works without breaking any other existing test!

Do we even need to raise a warning? There is no default index created for any nD coordinate variable anyway...

dcherian and others added 2 commits July 16, 2023 10:01
@dcherian
Remove warning
1f92e30
@dcherian @benbovy
Add invariants check
f059e50 
Co-authored-by: Benoit Bovy <benbovy@gmail.com>
@dcherian
Copy link
Contributor Author

Wow I'm positively surprised that it works without breaking any other existing test!

Only broke two tests that look for the error.

Do we even need to raise a warning? There is no default index created for any nD coordinate variable anyway...

👍 removed the error.

The internal invariants assert fails on main but succeeds on #7368

I updated the definition of "default indexes" to only check for 1D index vars, so I'm a little more confident now. Some of that code was copied from #7368

dcherian added 2 commits July 16, 2023 10:40
@dcherian
Merge branch 'main' into warn-nd-index-var
65c658b 
* main:
  Remove hue_style from plot1d docstring (pydata#7925)
  Add new what's new section (pydata#7986)
  Release summary for v2023.07.0 (pydata#7979)
  Improve explanation in example "Working with Multidimensional Coordinates" (pydata#7984)
  Fix typo in zarr.py (pydata#7983)
  Examples added to docstrings  (pydata#7936)
  [pre-commit.ci] pre-commit autoupdate (pydata#7973)
  Skip broken tests on python 3.11 and windows (pydata#7972)
  Use another repository for upstream testing (pydata#7970)
  Move absolute path finder from open_mfdataset to own function (pydata#7968)
  ensure no forward slashes in names for HDF5-based backends (pydata#7953)
  Chunked array docs (pydata#7951)
  [pre-commit.ci] pre-commit autoupdate (pydata#7959)
  manually unshallow the repository on RTD (pydata#7961)
  Update minimum version of typing extensions in pre-commit (pydata#7960)
  Docstring examples (pydata#7881)
@dcherian
Add whats-new
ba95097
@dcherian dcherian marked this pull request as ready for review July 16, 2023 15:44
@benbovy
Copy link
Member

benbovy commented Jul 17, 2023

LGTM, thanks @dcherian!

@dcherian dcherian added the plan to merge Final call for comments label Jul 17, 2023
@benbovy benbovy mentioned this pull request Jul 18, 2023
Open
49 tasks
@dcherian dcherian changed the title Warn instead of raising when automatically creating nD index variable Allow opening datasets with nD dimenson coordinate variables. Jul 18, 2023
@dcherian dcherian merged commit 2bf15f8 into pydata:main Jul 19, 2023
@dcherian dcherian deleted the warn-nd-index-var branch July 19, 2023 18:25
@dcherian
Copy link
Contributor Author

Finally!

cc @kthyng @ChrisBarker-NOAA @rsignell

@ChrisBarker-NOAA
Copy link

Awesome, thanks!!

SciPy sprints really have their benefits :-)

@ChrisBarker-NOAA ChrisBarker-NOAA mentioned this pull request Jul 19, 2023
Open
dcherian added a commit to dcherian/xarray that referenced this pull request Jul 24, 2023
@dcherian
Merge remote-tracking branch 'upstream/main' into groupby-binary-ops-…
3bb998e 
…lazy-array

* upstream/main: (153 commits)
  Add HDF5 Section to read/write docs page (pydata#8012)
  [pre-commit.ci] pre-commit autoupdate (pydata#8014)
  Update interpolate_na in dataset.py (pydata#7974)
  improved docstring of to_netcdf (issue pydata#7127) (pydata#7947)
  Expose "Coordinates" as part of Xarray's public API (pydata#7368)
  Core team member guide (pydata#7999)
  join together duplicate entries in the text `repr` (pydata#7225)
  Update copyright year in README (pydata#8007)
  Allow opening datasets with nD dimenson coordinate variables. (pydata#7989)
  Move whats-new entry
  [pre-commit.ci] pre-commit autoupdate (pydata#7997)
  Add documentation on custom indexes (pydata#6975)
  Use variable name in all exceptions raised in `as_variable` (pydata#7995)
  Bump pypa/gh-action-pypi-publish from 1.8.7 to 1.8.8 (pydata#7994)
  New whatsnew section
  Remove future release notes before this release
  Update whats-new.rst for new release (pydata#7993)
  Remove hue_style from plot1d docstring (pydata#7925)
  Add new what's new section (pydata#7986)
  Release summary for v2023.07.0 (pydata#7979)
  ...
@keewis keewis mentioned this pull request Aug 16, 2023
Closed
4 tasks
@blaylockbk blaylockbk mentioned this pull request Aug 23, 2023
Closed
@benbovy benbovy mentioned this pull request Sep 18, 2023
Open
4 tasks
@dcherian dcherian mentioned this pull request Nov 30, 2023
Closed
@benbovy benbovy mentioned this pull request Mar 28, 2024
Closed
5 tasks
@benbovy benbovy mentioned this pull request Mar 28, 2024
Closed
4 tasks
@TomNicholas TomNicholas mentioned this pull request Mar 28, 2024
Merged
4 tasks
@danielfromearth danielfromearth mentioned this pull request Aug 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benbovy benbovy Awaiting requested review from benbovy

Assignees
No one assigned
Labels
plan to merge Final call for comments topic-indexing topic-testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problem opening unstructured grid ocean forecasts with 4D vertical coordinates
3 participants
@dcherian @benbovy @ChrisBarker-NOAA