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

Use zarr-fixture to prevent thread leakage errors #9967

Merged
merged 6 commits into from
Jan 22, 2025
Merged

Use zarr-fixture to prevent thread leakage errors #9967

merged 6 commits into from
Jan 22, 2025

Conversation

kmuehlbauer
Copy link
Contributor

@kmuehlbauer kmuehlbauer commented Jan 21, 2025 •
edited
Loading

  • Test fixed
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

@kmuehlbauer
Copy link
Contributor Author

@jhamman, is this the right way to fix this for xarray? I've vendored the respective fixture and left a note, but is this the correct way of doing that?

@kmuehlbauer kmuehlbauer mentioned this pull request Jan 21, 2025
Merged
1 task
jhamman
jhamman approved these changes Jan 22, 2025
View reviewed changes
Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

A nit and and small comment. But This looks good to me.

xarray/tests/test_distributed.py
Comment on lines +230 to +231
_ = zarr_lib.core.sync._get_loop()
_ = zarr_lib.core.sync._get_executor()
Copy link
Member

Choose a reason for hiding this comment

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

Eyes wide open here. This is quite private API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to the note, thanks for making this clear!

@kmuehlbauer
Copy link
Contributor Author

kmuehlbauer commented Jan 22, 2025 •
edited
Loading

Issues with new versions of dependencies slowly trickling in now. As of dask 2025.1.0 missing pyarrow will raise ImportError when importing dask.dataframe. On linux/macos this is somehow pulled in by other dependencies, but not on windows. I've added pyarrow to the respective windows envronments in #9971.

@kmuehlbauer kmuehlbauer mentioned this pull request Jan 22, 2025
Merged
3 tasks
@kmuehlbauer kmuehlbauer added the run-upstream Run upstream CI label Jan 22, 2025
@kmuehlbauer
Copy link
Contributor Author

Beside the typing issues CI is green again. 🎉

@kmuehlbauer kmuehlbauer merged commit 609412d into pydata:main Jan 22, 2025
33 of 35 checks passed
@kmuehlbauer kmuehlbauer deleted the fix-dask-zarr-integration-test branch January 22, 2025 09:18
dcherian added a commit that referenced this pull request Jan 30, 2025
@dcherian
Merge branch 'main' into backend-indexing
dffc05c 
* main: (79 commits)
  fix mean for datetime-like using the respective time resolution unit (#9977)
  Add `time_unit` argument to `CFTimeIndex.to_datetimeindex` (#9965)
  remove gate and add a test (#9958)
  Remove repetitive that (replace it with the) (#9994)
  add shxarray to the xarray ecosystem list (#9995)
  Add `shards` to `valid_encodings` to enable sharded Zarr writing (#9948)
  Use flox for grouped first, last (#9986)
  Bump the actions group with 2 updates (#9989)
  Fix some typing (#9988)
  Remove unnecessary a article (#9980)
  Fix test_doc_example on big-endian systems (#9949)
  fix weighted polyfit for arrays with more than 2 dimensions (#9974)
  Use zarr-fixture to prevent thread leakage errors (#9967)
  remove dask-expr from CI runs, fix related tests (#9971)
  Update time coding tests to assert exact equality (#9961)
  cast type to PDDatetimeUnitOptions (#9963)
  Suggest the correct name when no key matches in the dataset (#9943)
  fix upstream dev issues (#9953)
  Relax nanosecond datetime restriction in CF time decoding (#9618)
  Remove outdated quantile test. (#9945)
  ...
dcherian added a commit that referenced this pull request Jan 30, 2025
@dcherian
Merge branch 'main' into backend-indexing
1ffe5e9 
* main: (79 commits)
  fix mean for datetime-like using the respective time resolution unit (#9977)
  Add `time_unit` argument to `CFTimeIndex.to_datetimeindex` (#9965)
  remove gate and add a test (#9958)
  Remove repetitive that (replace it with the) (#9994)
  add shxarray to the xarray ecosystem list (#9995)
  Add `shards` to `valid_encodings` to enable sharded Zarr writing (#9948)
  Use flox for grouped first, last (#9986)
  Bump the actions group with 2 updates (#9989)
  Fix some typing (#9988)
  Remove unnecessary a article (#9980)
  Fix test_doc_example on big-endian systems (#9949)
  fix weighted polyfit for arrays with more than 2 dimensions (#9974)
  Use zarr-fixture to prevent thread leakage errors (#9967)
  remove dask-expr from CI runs, fix related tests (#9971)
  Update time coding tests to assert exact equality (#9961)
  cast type to PDDatetimeUnitOptions (#9963)
  Suggest the correct name when no key matches in the dataset (#9943)
  fix upstream dev issues (#9953)
  Relax nanosecond datetime restriction in CF time decoding (#9618)
  Remove outdated quantile test. (#9945)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhamman jhamman jhamman approved these changes

Assignees
No one assigned
Labels
run-upstream Run upstream CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kmuehlbauer @jhamman