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.

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

Ensure inline_array keeps attributes #354

Merged
merged 5 commits into from
Aug 25, 2023

Conversation

jrbourbeau
Copy link
Contributor

Currently inline_array won't keep attrs around for arrays that it inlines. This PR makes sure attrs are preserved.

cc @martindurant

@martindurant
Copy link
Member

We are hitting a hard crash here (on shutdown of the pytest session), which I can only imagine is coming from HDF5 or maybe grib. This will take some debugging...

@jrbourbeau
Copy link
Contributor Author

Just checking, my guess is this failure is happening on main too. I don't see how that could be related to the changes here (though maybe I'm missing something)

@martindurant
Copy link
Member

this failure is happening on main too

Not locally :|

@jrbourbeau
Copy link
Contributor Author

I just updated the test added here to use a more simple attrs. This seemed to help locally, let's see if it helps in CI. I don't know why the previous attrs were problematic, but the simplified attrs should still be sufficient

@martindurant
Copy link
Member

Since it's OK for py38 but crashed on py310, suggests a version problem. I note h5py 3.9.0 just came out.

@jrbourbeau jrbourbeau mentioned this pull request Aug 25, 2023
@martindurant
Copy link
Member

Confirmed: pinning h5py makes tests pass https://github.com/fsspec/kerchunk/actions/runs/5978456412/job/16220596021?pr=349

@martindurant
Copy link
Member

^ cc @ajelenak

@martindurant
Copy link
Member

Pushing the temp fix here, and we can deal with whatever is the underlying problem separately.

@martindurant martindurant merged commit 9c6309b into fsspec:main Aug 25, 2023
5 checks passed
@jrbourbeau jrbourbeau deleted the inline-array-attrs branch August 25, 2023 18:12
@jrbourbeau
Copy link
Contributor Author

Thanks @martindurant

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.

2 participants