Skip to content

Conversation

@niksirbi
Copy link
Member

@niksirbi niksirbi commented Oct 21, 2025

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

tests/test_integration/test_netcdf.py raises in CI raises repeated errors of the form:

RuntimeError: NetCDF: HDF error
OSError: [Errno -101] NetCDF: HDF error: '/tmp/pytest-of-runner/pytest-0/test_ds_save_and_load_netcdf_hX/test_dataset.nc'

These are due to a failing test_ds_save_and_load_netcdf test, specifically when the netcdf4 or the h5netcdf engines are used (scipy engine is fine).

See example failing CI run here.

I'm unable to reproduce the test failures locally, and I suspect the CI is using a different xarray version than I have locally.

According to the xarray release notes, they recently changed the default engine order in v2025.09.01 and reverted it in v2025.10.1, because it caused tons of issues. See references for some relevant issues/PRs.

What does this PR do?

Trying around various workarounds.

References

How has this PR been tested?

CI and running tox locally.

Is this a breaking change?

No.

Does this PR require an update to the documentation?

No.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@niksirbi niksirbi mentioned this pull request Oct 21, 2025
7 tasks
@codecov
Copy link

codecov bot commented Oct 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (f9d728e) to head (b567cf6).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #685   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           34        34           
  Lines         1997      1997           
=========================================
  Hits          1997      1997           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@niksirbi niksirbi force-pushed the fix-failing-netcdf-tests branch from ae7f14c to eafbe40 Compare October 26, 2025 08:28
@sonarqubecloud
Copy link

@niksirbi
Copy link
Member Author

niksirbi commented Oct 27, 2025

It turns out the issue wasn’t with xarray after all.
Recent PyPI releases of netCDF4 (a dependency of xarray[io]) and h5py appear to ship with mutually incompatible wheels, which leads to the NetCDF: HDF error we were seeing.

For now, the problem can be worked around by upper-pinning netCDF4, and I’ve opened issue #687 to track this. That issue includes relevant background and links to upstream reports.

An alternative would be to install all HDF5-related packages via conda-forge in CI, which would likely solve the problem too. However, that approach would effectively require us to recommend conda-forge to all users and would undermine the recent efforts to support a pure pip/uv installation workflow.

Upper-pinning netCDF4 isn’t ideal, but it resolves the CI failures for now and works cleanly with pip/uv installs, as verified in PR #683. Hopefully we can relax the pin once compatible wheels are released upstream.

@niksirbi niksirbi marked this pull request as ready for review October 27, 2025 13:39
@niksirbi niksirbi requested a review from lochhh October 27, 2025 13:39
@niksirbi niksirbi changed the title Fix failing netCDF integration tests Pin netcdf<1.7.3 to fix failing integration tests Oct 27, 2025
Copy link
Collaborator

@lochhh lochhh left a comment

Choose a reason for hiding this comment

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

Thanks for the detective work @niksirbi.

Just some additional notes on CI runs with different netCDF4 versions built on different netCDF-c libraries:

Platform Python Status netCDF4 Version netCDF-c Library Version
windows py13 passing 1.7.2 4.9.2 of Oct 4 2024 15:30:23
mac py13 passing 1.7.2 4.9.2 of Mar 14 2023 20:34:25
ubuntu py13 passing 1.7.2 4.9.4-development of Oct 7 2024 08:34:05
ubuntu py12 passing 1.7.2 4.9.4-development of Oct 7 2024 08:34:05
ubuntu py11 passing 1.7.2 4.9.4-development of Oct 7 2024 08:34:05
windows py13 passing 1.7.3 4.9.2 of Aug 1 2025 13:27:44
mac py13 passing 1.7.3 4.9.3 of Feb 7 2025 21:40:00
ubuntu py13 failing 1.7.3 4.9.3 of Aug 4 2025 11:49:07
ubuntu py11 failing 1.7.3 4.9.3 of Aug 4 2025 11:49:07
ubuntu py12 failing 1.7.3 4.9.3 of Aug 4 2025 11:49:07

@niksirbi
Copy link
Member Author

Just some additional notes on CI runs with different netCDF4 versions built on different netCDF-c libraries:

Amazing, thanks @lochhh. Great to have this table for future reference.

@niksirbi niksirbi added this pull request to the merge queue Oct 29, 2025
Merged via the queue into main with commit 5ef3dbc Oct 29, 2025
27 checks passed
@lochhh lochhh deleted the fix-failing-netcdf-tests branch October 29, 2025 12:37
animeshsasan pushed a commit to animeshsasan/movement that referenced this pull request Nov 16, 2025
…nit#685)

* read netCDF file with the engine used for saving them.

* Revert "read netCDF file with the engine used for saving them."

This reverts commit c8f382e.

* exclude problematic versions of xarray

* pin xarray version

* try upper-pinning h5py and netcdf4

* upper pin only on linux

* try upper-pinning only netCDF4

* also try without tables pip dependency
github-merge-queue bot pushed a commit that referenced this pull request Nov 17, 2025
* Fixing Movement Build step

* Testing movement docs deployment

* Update contributors list

* Update links to SLEAP docs (#681)

* improve resulting displacement vectors (#657)

* cart2pol now always sets phi=0 when rho=0

* Improve displace vectors computation

* deprecate movement.kinematics.compute_displacement
* add movement.kinematics.compute_forward_displacement
* add movement.kinematics.compute_backward_displacement
* adapt examples and docstrings accordingly
* adapt tests to the implementation changes

* update compute_kinematics example

shows the existance of compute_path_length

* Add backward displacement to the example. Small edits to wording and flow

* Fix double ticks

* Deprecation warning after short summary following numpydoc style

https://numpydoc.readthedocs.io/en/latest/format.html#deprecation-warning

* Factor out deprecation test for kinematics as a general test

* Clarify handling of unsigned zeros in cart2pol

* Fix rendering bits

* Fix broken SLEAP link

---------

Co-authored-by: sfmig <[email protected]>

* [pre-commit.ci] pre-commit autoupdate (#682)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.12.11 → v0.13.3](astral-sh/ruff-pre-commit@v0.12.11...v0.13.3)
- [github.com/pre-commit/mirrors-mypy: v1.17.1 → v1.18.2](pre-commit/mirrors-mypy@v1.17.1...v1.18.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* update links to animovement (#684)

* Pin netcdf<1.7.3 to fix failing integration tests (#685)

* read netCDF file with the engine used for saving them.

* Revert "read netCDF file with the engine used for saving them."

This reverts commit c8f382e.

* exclude problematic versions of xarray

* pin xarray version

* try upper-pinning h5py and netcdf4

* upper pin only on linux

* try upper-pinning only netCDF4

* also try without tables pip dependency

* Add documentation to specify Movement require identical keypoints (#150) (#658)

* Add documentation to specify that skeleton must be identical for all individuals

* Fixed tabs displaying

* Fixe tabs displaying

* Update docs/source/user_guide/input_output.md

Co-authored-by: Niko Sirmpilatze <[email protected]>

* remove note from from_file()

* Update movement/io/load_poses.py

Co-authored-by: Niko Sirmpilatze <[email protected]>

---------

Co-authored-by: Niko Sirmpilatze <[email protected]>

* [pre-commit.ci] pre-commit autoupdate (#692)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.13.3 → v0.14.3](astral-sh/ruff-pre-commit@v0.13.3...v0.14.3)
- [github.com/mgedmin/check-manifest: 0.50 → 0.51](mgedmin/check-manifest@0.50...0.51)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update contributors list (#690)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Bump actions/download-artifact from 5 to 6 (#691)

Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 5 to 6.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v5...v6)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert changes to main

* Add function for computing homography transform

* Fix unit tests and improve validation

* Resolve comments and add more tests

* Minor fixes as per comments

* Apply suggestion from @niksirbi

Co-authored-by: Niko Sirmpilatze <[email protected]>

* remove duplicate entry from people

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Niko Sirmpilatze <[email protected]>
Co-authored-by: carlocastoldi <[email protected]>
Co-authored-by: sfmig <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: CeliaLrt <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants