a high-level comment about the conversion strategy. this is not a blocker for this PR, but the current conversion strategy is serial over arrays and groups. This means converting an array / group waits until the previous one is converted, and it increases the likelihood of conversion breaking in the middle, leaving a hybrid v2 / v3 hierarchy.

We could also do conversion with a concurrent API based on concurrently reading the entire input hierarchy withdict(group.members(max_depth=None)), preparing new metadata documents for each member, then concurrently writing those documents withcreate_hierarchy.

This could be considered merely a performance concern, but I suspect reading and writing the hierarchy concurrently might be the difference between success and failure for this CLI when people use it on high-latency storage backends.

@K-Meech if you want to experience how your CLI tool handles IO latency, we have astore wrapper class that adds latency to any other store class. Try running your conversion on a large zarr hierarchy with 100ms of latency added toget andset on a local store. If it takes more than a few seconds, then this strongly argues for moving away from the serial conversion strategy and using something concurrent.

Thanks@d-v-b . I tried converting a Zarr with 30 nested groups (each with a small array inside) and this took about 10 seconds total (with aLatencyStore set up asLatencyStore(local_store, get_latency=0.1, set_latency=0.1)). I'vecreated a gist of this small test, if you wanted to try it out.

Happy to look at changing this to use concurrent writes withcreate_hierachy - perhaps best to do in a separate PR after this one is merged? What do you think?

I've been keeping a list of potential follow up changes mentioned on this PR - happy to convert these into issues after merge:

• Handle conversion of consolidated metadata (.zmetadata) files
• Handle any combination ofnumcodecs codecs -see discussion in thread. Currently the code only handles filters that areArrayArrayCodecs and compressors that areBytesBytesCodecs.
• Use concurrent writes, rather than serial

Thanks@d-v-b . I tried converting a Zarr with 30 nested groups (each with a small array inside) and this took about 10 seconds total (with aLatencyStore set up asLatencyStore(local_store, get_latency=0.1, set_latency=0.1)). I'vecreated a gist of this small test, if you wanted to try it out.

cool, thanks for running this experiment! IMO 10 seconds for converting 30 arrays and 30 groups is fine to start with. We can treat the performance tuning as an implementation detail for a later PR.

I've fixed the merge conflicts with this branch, and updated docstrings to reference the newzarr.codecs.numcodecs (rather thannumcodecs.zarr3). I think I've addressed everyone's comments as well -@dstansby , see my replies to your comments above.
Do let me know if anything else needs changing / updating here - otherwise it might be ready to go in?

@zarr-developers/python-core-devs I would like to merge this soon (24 hours) so now is a great time to look over this PR!

it's going in once the tests are green. Thanks so much@K-Meech! If you don't mind, I'd like to make a post about your work over at image.sc, so people know about this functionality. This might be a good way to get some early feedback from users.

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 3.1.xgit pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 3c883a3c578b6e9fdb4d5fd7a160ce992aaec1b3

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #3257: Add CLI for converting v2 metadata to v3'

4. Push to a named branch:

git push YOURFORK 3.1.x:auto-backport-of-pr-3257-on-3.1.x

5. Create a PR against branch 3.1.x, I would have named this PR:

"Backport PR#3257 on branch 3.1.x (Add CLI for converting v2 metadata to v3)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove theStill Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free tosuggest an improvement.

Great - thanks@d-v-b ! I'll make some follow up issues foradditional features from the reviews on this PR.

👏🏽 👏🏽 - The one thought I had while pondering this,@K-Meech, (and sorry if it was already discussed) is to what degree the CLI will follow the semver of the library itself. Not that anything needs doing but might be worth expectation management on the part of consumers.

👏🏽 👏🏽 - The one thought I had while pondering this,@K-Meech, (and sorry if it was already discussed) is to what degree the CLI will follow the semver of the library itself. Not that anything needs doing but might be worth expectation management on the part of consumers.

the library doesn't follow semver -- we are usingeffver

Fair enough, effver then. Can a user at this point reasonably expect the same level of stability from the CLI that they would for the API?

I don't see why we should version the CLI any differently, other than the fact that it's pretty new and so it might need some polishing as people use it. To give ourselves some flexibility, we could consider adding a disclaimer to the CLI stating that it's still experimental and might experience breaking changes in future versions of zarr python.

see#3465