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

Nested sandbox.zarr in 00_xenium_and_visium.ipynb #36

Closed
melonora opened this issue Jun 26, 2023 · 8 comments
Closed

Nested sandbox.zarr in 00_xenium_and_visium.ipynb #36

melonora opened this issue Jun 26, 2023 · 8 comments

Comments

@melonora
Copy link
Collaborator

The LANDMARKS_SDATA_PATH is set to GENERATED_DATA_PATH / "sandbox.zarr" in the notebook. However when downloading and unzipping the data the sandbox.zarr directory contains sandbox.zarr itself. This can cause users who don't know this to get an error as the upper level is not a valid .zarr.

@kevinyamauchi
Copy link
Contributor

@LucaMarconato , what is your preferred fix here? I think just adding a note about this in a markdown cell would be sufficient. Thoughts?

@LucaMarconato
Copy link
Member

LucaMarconato commented Jun 27, 2023

@melonora thanks for reporting.

This is the intended behavior, the user should place the sandbox.zarr file in the path GENERATED_DATA, which is spatialdata-sandbox/generated_data/xenium_visium_integration.

In the notebooks it's written to make this path discoverable via symlinks, so as a user I think I would be aware that I need to place the unzipped file to the right place.

Anyway, to avoid confusion, I will add another note in the notebook when defining GENERATED_DATA_PATH / "sandbox.zarr" saying to the user that the files needs to be placed to the right location.

Edit see below.

@LucaMarconato
Copy link
Member

But just to be sure that I understood your concern correctly, here also the BC_atlas_we.h5ad and visium_copyKat.h5ad needs to be placed to the right location.

image

Why do you think that extra care needs to be placed to sandbox.zarr?

@LucaMarconato
Copy link
Member

Ah! I think I get it! One may expect that sandbox.zarr would expand to spatialdata-sandbox, but sandbox.zarr was a bad chosen name that I never changed thereafter. A better name is manual_annotations.zarr. I will open an issue to rename the file.

@LucaMarconato
Copy link
Member

LucaMarconato commented Jun 27, 2023

I opened the issue here (and added extra details): #39, I close this issue because the problem is not about nesting but about a poorly chosen name.

@melonora
Copy link
Collaborator Author

melonora commented Jun 28, 2023

it might be poorly choosen as well, but right now also when downloading you get this:
sandbox.zarr/sandbox.zarr where the top level sandbox.zarr is not a valid zarr and you indicate in the notebook that this is the level that will be read as zarr file, which causes an error. It is a minor thing:) For the record this is what you get:
afbeelding

@kevinyamauchi
Copy link
Contributor

kevinyamauchi commented Jun 28, 2023

For the record this is what you get:

Yeah, I've seen that as well. For now, I think we just need a simple comment in the notebook explaining the issue.

@LucaMarconato
Copy link
Member

Ah ok I understand now. I can't reproduce this with macOS (both extracting using the finder or using unzip). I think it's Windows specific but I will now add a note in the notebook about that.

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

No branches or pull requests

3 participants