Skip to content

Always strip preceding / from path in ManifestStore#764

Merged
maxrjones merged 2 commits intomainfrom
improve-prefix-stripping
Aug 9, 2025
Merged

Always strip preceding / from path in ManifestStore#764
maxrjones merged 2 commits intomainfrom
improve-prefix-stripping

Conversation

@maxrjones
Copy link
Member

@maxrjones maxrjones commented Aug 8, 2025

Obstore expects the preceding '/' to be stripped.

  • Closes Test failures with pytest path #761
  • Tests added
  • Tests passing
  • Full type hint coverage
  • Changes are documented in docs/releases.rst
  • New functions/methods are listed in api.rst
  • New functionality has documentation

@codecov
Copy link

codecov bot commented Aug 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.55%. Comparing base (be9e1df) to head (28d00f7).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #764   +/-   ##
=======================================
  Coverage   87.55%   87.55%           
=======================================
  Files          35       35           
  Lines        1848     1848           
=======================================
  Hits         1618     1618           
  Misses        230      230           
Files with missing lines Coverage Δ
virtualizarr/manifests/store.py 87.71% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maxrjones maxrjones requested a review from a team August 8, 2025 19:45
Copy link
Member

@TomNicholas TomNicholas left a comment

Choose a reason for hiding this comment

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

Did obstore's behaviour change? Why did this only just start failing?

@maxrjones
Copy link
Member Author

Did obstore's behaviour change? Why did this only just start failing?

Yes, v0.8.0 included a breaking change that requires users to ensure paths are valid - https://developmentseed.org/obstore/latest/CHANGELOG/#whats-changed

@maxrjones maxrjones merged commit 1e632c3 into main Aug 9, 2025
15 checks passed
@maxrjones maxrjones deleted the improve-prefix-stripping branch August 9, 2025 17:08
maxrjones added a commit that referenced this pull request Aug 9, 2025
* Always strip preceding / from path in ManifestStore

* Fix docs error
TomNicholas added a commit that referenced this pull request Aug 14, 2025
* Unpin xarray

* Fix Icechunk tests with xarray 2025.07.0

* Simplify test_set_grid_virtual_refs

* Always strip preceding / from path in ManifestStore

* Fix docs error

* Consolidate into fixture

* Simplify some append icechunk tests

* Always strip preceding / from path in ManifestStore (#764)

* Always strip preceding / from path in ManifestStore

* Fix docs error

* Add xarray to upstream

* Refactor icechunk append tests

* Update offset in DMR++ tests

* Update virtualizarr/tests/test_writers/test_icechunk.py

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* use same minimum version of xarray that we used before pinning it

* add release note

* move new test fixtures to conftest.py

* move fixture I missed

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Tom Nicholas <tom@earthmover.io>
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.

Test failures with pytest path

2 participants