Skip to content

Conversation

@willirath
Copy link
Collaborator

@willirath willirath commented Feb 5, 2023

Let's allow for directly passing a zarr.storage.Store object to ParticleFile. This will allow for a couple of advanced output strategies:

  • diskless capture of the output into a MemoryStore
  • streaming data to a Redis store using RedisStore
  • capturing output in a zip file using a ZipZtore (Possible but inefficient and throwing lots of warnings.)
  • etc.

Todos:

  • make sure ParticleFile works for Zarr stores directly.
  • document changes
  • add tutorial / example notebook with some of the above stores

@willirath willirath marked this pull request as ready for review February 5, 2023 19:37
@erikvansebille
Copy link
Member

Thanks for this PR, @willirath, it looks very good!

Apart from the small change proposed above on the MPI-handling, my only comment would be to rename the tutorial_zarr_advanced notebook to documentation_advanced_zarr. In Parcels, documentations are typically more advanced and/or for background, and they are not integration-tested. Since you already have a unit-test too (thanks!), I'm not sure integration testing is needed too. Or do you think that adds value?

Also, I'm pinging @JamiePringle, so he can have a quick look at this new feature too

@willirath
Copy link
Collaborator Author

@erikvansebille I've renamed to notebook as suggested and also fixed the MPI check.

Re this also being a possible integration test: I think it wouldn't hurt as it also does not rely on any input data (but produces some output) and is relatively quick (30 seconds in total?). But seeing the already substantial time spent on automated tests, I can also see that it makes sense not to run everything.

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