Conversation
pyproject.toml
Outdated
| dynamic = ["version"] | ||
| dependencies = [ | ||
| "xarray", | ||
| "xarray@git+https://github.com/TomNicholas/xarray#egg=concat-no-indexes", |
There was a problem hiding this comment.
This was required to get a few tests to pass. Even with this change I am still getting the error
E NotImplementedError: ManifestArrays can't be converted into numpy arrays or pandas Index objects
from virtualizarr/tests/test_xarray.py:175 TestConcat.test_concat_dim_coords_along_existing_dim
There was a problem hiding this comment.
The CI in this PR is not using the correct branch of xarray - the CI shows the error is raised on this line
result = type(datasets[0])(result_vars, attrs=result_attrs)but in pydata/xarray#8872 and my concat-no-indexes branch this line is different
the line should read
result = type(datasets[0])(result_vars, coords=coords, attrs=result_attrs)so something is up with the pip-installing of that specific branch.
(I'm trying to get pydata/xarray#8872 merged so that we can just use xarray main)
There was a problem hiding this comment.
Also note that this test passes on the CI on main of this package
https://github.com/TomNicholas/VirtualiZarr/actions/runs/8457581269/job/23169875291
There was a problem hiding this comment.
ok trying with the default xarray branch, like in the main branch of this package.
|
Thanks for the issue and for the fix @abarciauskas-bgse ! I think this PR is mergeable, but the tests are failing because the wrong version of xarray is being installed by the CI... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
==========================================
+ Coverage 90.29% 90.39% +0.09%
==========================================
Files 14 14
Lines 938 947 +9
==========================================
+ Hits 847 856 +9
Misses 91 91
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Tests are now passing @TomNicholas, let me know if you have other comments 👍🏽 |
|
This looks great, thank you! |
Addresses #65