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

Improve Zarr Import UI (and general editing of datasets) #6485

Merged
merged 45 commits into from
Oct 17, 2022
Merged

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Sep 21, 2022

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • In general do some monkeytesting with the following:
    • use new "Add Zarr UI" (e.g., with layer url == https://data-humerus.webknossos.org/data/zarr/scalable_minds/l4dense_motta_et_al_demo/segmentation)
    • edit an existing dataset (zarr or wkw)

Issues:


(Please delete unneeded items, merge only when none are left open)

@philippotto philippotto self-assigned this Sep 21, 2022
@philippotto
Copy link
Member Author

[ ] The default name for dataset is the same as the layer name of the first layer. I am not sure if this makes sense. In case of your test DS above, the dataset name should have been l4_dense_motta. I am not sure if the name can also be derived from the Zarr file/spec or with should either leave the field empty or with a generic name.

@fm3 I think the initial name is coming from the back-end, right? defaulting to an empty string sounds like a good solution for now (definitely, better than a poor guess, I'd say).

@philippotto
Copy link
Member Author

  • Maybe not technically caused by this PR but wK offers a config suggestion, i.e. to remove all datalayer (dataLayer = []) when clearly the advanced view still has content/values for this property...

@fm3 I fixed this in the front-end now. I think it's fair to say that the backend only infers what's on disk. So, adding existing zarr layers to the inferred datasource makes (some) sense to me. Let me know in case you disagree 🤙

@fm3
Copy link
Member

fm3 commented Oct 6, 2022

backend only infers what's on disk

Yes, makes sense to me 👍

initial name is coming from the back-end, right?

correct

defaulting to an empty string sounds like a good solution for now (definitely, better than a poor guess, I'd say).

ok I’ll include that change in #6520 to avoid merge conflicts, that pr should be merged soon

@philippotto
Copy link
Member Author

@hotzenklotz Thanks for the thorough testing. I debugged and integrated your ideas as best as possible. Please test again (the new dev instance is already deployed).

Another nitpick: There is no easy way to add layers to an existing dataset later. Yes, I can manually add a layer to the datasource through the advanced JSON field but that is error prone. (I tried adding a Zarr color layer, which required me to remember the "magic"/"hidden" properties, "dataFormat": "zarr". The URL was also incorrect in my case

This is the only thing I didn't do. The form handling with antd is quite error-prone. I'd defer this feature to another iteration (maybe in combination to when the dataset management topic is tackled) to not drag this PR any longer.

@hotzenklotz
Copy link
Member

I re-tested and LGTM. Ready to merge :-)

@philippotto philippotto enabled auto-merge (squash) October 17, 2022 13:35
@philippotto
Copy link
Member Author

I re-tested and LGTM. Ready to merge :-)

I need your official acceptance to merge this PR :)

@philippotto philippotto merged commit c4dd3a4 into master Oct 17, 2022
@philippotto philippotto deleted the zarr-form branch October 17, 2022 17:44
hotzenklotz added a commit that referenced this pull request Oct 18, 2022
…ebar_button

* 'master' of github.com:scalableminds/webknossos:
  added more usage shortcuts to status bar (#6549)
  Look up annotation dataset by tracing id instead of (broken) organizationName field (#6548)
  Add links to resource creation pages (#6513)
  Improve Zarr Import UI (and general editing of datasets) (#6485)
  Remove unused dataSetName and organizationName property of protobuf objects (#6559)
  WIP: Added manual task assignment (#6551)
  Fix zarr private links for volume tracings and some minor improvements to private links UI (#6541)
  Fix stack overflow in compactMovedNodesAndEdges (#6557)
hotzenklotz added a commit that referenced this pull request Oct 18, 2022
…ect_spinner

* 'master' of github.com:scalableminds/webknossos:
  added more usage shortcuts to status bar (#6549)
  Look up annotation dataset by tracing id instead of (broken) organizationName field (#6548)
  Add links to resource creation pages (#6513)
  Improve Zarr Import UI (and general editing of datasets) (#6485)
  Remove unused dataSetName and organizationName property of protobuf objects (#6559)
  WIP: Added manual task assignment (#6551)
  Fix zarr private links for volume tracings and some minor improvements to private links UI (#6541)
  Fix stack overflow in compactMovedNodesAndEdges (#6557)
hotzenklotz added a commit that referenced this pull request Oct 19, 2022
…ault_volume_layer

* 'master' of github.com:scalableminds/webknossos: (21 commits)
  adds ETA calculation to voxelytics tasks (#6564)
  update immutability-helper package to v3.1.1 with TS support (#6565)
  [Skeletons] Allow deletion of root group (#6553)
  fix disappearing sidebar button (#6555)
  Enable "What's new" in navbar for everybody (#6563)
  Highlight Menu in Navbar Menu (#6558)
  refactor Select loading spinner property (#6556)
  Reload histogram when reloading layer (#6537)
  added more usage shortcuts to status bar (#6549)
  Look up annotation dataset by tracing id instead of (broken) organizationName field (#6548)
  Add links to resource creation pages (#6513)
  Improve Zarr Import UI (and general editing of datasets) (#6485)
  Remove unused dataSetName and organizationName property of protobuf objects (#6559)
  WIP: Added manual task assignment (#6551)
  Fix zarr private links for volume tracings and some minor improvements to private links UI (#6541)
  Fix stack overflow in compactMovedNodesAndEdges (#6557)
  Add backspace keyboard shortcut to delete active node (#6554)
  fix scaling in new meshfile rendering (#6552)
  Added more filters and sorting options to Projects list (#6550)
  fix double occurance of "ago" in VX progress times (#6535)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve datasource editing for remote zarr import
3 participants