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.

Sign up for GitHub

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

Jump to bottom

Merge editable mappings #7026

Merged
merged 33 commits into from
May 10, 2023
Merged

Merge editable mappings #7026

merged 33 commits into from
May 10, 2023

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Apr 26, 2023 •
edited
Loading

  • Adds merge functionality for editable mapping (a.k.a. supervoxel proofreading) volume layers
  • Fixes a bug in duplicating such annotations
  • Adds small refactoring Instant.since
  • Changes the merge modal from a checkbox to individual buttons to emphasize the difference between the options:
    image

TODO

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • While the new fossilDB is not released, this can be tested by checking out the FossilDB branch fix-get-multiple-keys-pagination, running sbt "run --columnFamilies skeletons,skeletonUpdates,volumeData,volumes,volumeUpdates,editableMappings,editableMappingUpdates,editableMappingsInfo,editableMappingsAgglomerateToGraph,editableMappingsSegmentToAgglomerate" there and yarn start noFossilDB in webknossos.
  • Create two explorative editable mapping annotations on the same dataset, merge/split some agglomerates
  • Use the merge modal to merge one with the other
  • Result should have updates of both mappings applied
  • Merge modal should be more understandable as well (feedback welcome)

Issues:

@fm3 fm3 self-assigned this Apr 26, 2023
@fm3 fm3 mentioned this pull request Apr 27, 2023
Closed
7 tasks
@fm3 fm3 marked this pull request as ready for review May 4, 2023 09:48
@fm3 fm3 requested a review from frcroth May 4, 2023 11:03
@fm3 fm3 requested a review from philippotto May 4, 2023 11:03
philippotto
philippotto reviewed May 4, 2023
View reviewed changes
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome 👍 I left some wording suggestions, but they might be too verbose or too on the nose. however, I still think, there is room to make the UI clearer. let me know what you think.

MIGRATIONS.unreleased.md Outdated
@@ -8,6 +8,7 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md).
## Unreleased
[Commits](https://github.com/scalableminds/webknossos/compare/23.05.1...HEAD)
- FossilDB needs to be opened with new additional column families editableMappingsInfo, editableMappingsAgglomerateToGraph, editableMappingsSegmentToAgglomerate.
- For instances with existing editable mapping (a.k.a supervoxel proofreading) annotations: To keep those annotations alive, a python migration has to be run with access to your tracingstore’s FossilDB. It is recommended to do this during a webknossos downtime to avoid data loss. It needs python 3.8+ and the pip packages installable by `pip install grpcio-tools grpcio-health-checking`. Run it with `python tools/migrate-editable-mappings/migrate-editable-mappings.py -v -w -o localhost:7155`. Omit -o for a faster migration but no access to older versions of the editable mappings. The migration is idempotent.
- For instances with existing editable mapping (a.k.a supervoxel proofreading) annotations: To keep those annotations alive, a python migration has to be run with access to your tracingstore’s FossilDB. It is recommended to do this during a webknossos downtime to avoid data loss. It needs python 3.8+ and the pip packages installable by `pip install grpcio-tools grpcio-health-checking`. Run it with `python tools/migrate-editable-mappings/migrate-editable-mappings.py -v -w -o localhost:7155`. Omit -o for a faster migration but no access to older versions of the editable mappings. The migration is idempotent.
- A new FossilDB Version is required [TODO: Insert release number an link here once fossilDB PR https://github.com/scalableminds/fossildb/pull/38 is merged]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo

frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/action-bar/merge_modal_view.tsx Show resolved Hide resolved
@fm3
Copy link
Member Author

fm3 commented May 4, 2023

I’d prefer not to change the text on the buttons. I had longer variants at first, but it made for a very cluttered UI and very large buttons and line breaks on small screens. I think with the tooltips, this should be ok. What do you think?

@philippotto
Copy link
Member

I’d prefer not to change the text on the buttons. I had longer variants at first, but it made for a very cluttered UI and very large buttons and line breaks on small screens. I think with the tooltips, this should be ok. What do you think?

ok, yes, I'm okay with that. one suggestion though: I'd only make one of the buttons primary (blue). the "import trees here" can probably get the default look, as it's a bit more specialized.

@fm3 fm3 requested a review from philippotto May 4, 2023 12:46
philippotto
philippotto reviewed May 4, 2023
View reviewed changes
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

front-end looks good to me 👍 i'm leaving the final acceptance to the backend review @frcroth

@fm3 fm3 merged commit f0208c6 into master May 10, 2023
@fm3 fm3 deleted the merge-editable-mappings-cherry branch May 10, 2023 08:52
hotzenklotz added a commit that referenced this pull request May 17, 2023
@hotzenklotz
Merge branch 'master' of github.com:scalableminds/webknossos into emp…
790a5bf 
…ty-list-drawings

* 'master' of github.com:scalableminds/webknossos: (23 commits)
  fix scrolling in organization switcher in terms-of-services modal (#7083)
  Auto-Select via SAM (#7051)
  Build STL in chunks when exporting them (#7074)
  Improve performance for large (oversegmentation) meshes (#7077)
  Fix display of used storage in power plan (#7057)
  Fix user limits for invites (#7078)
  adds fileSize to voxelytics workflow list (#7071)
  Improve error logging for bucket requests (#7053)
  Fix zarr streaming datasource-properties.json generation for non-wkw/zarr datasets (#7065)
  Min length for layer names is one (#7064)
  Include voxelytics workflow name in tab title (#7070)
  Fix local to global layer index look up (#7066)
  Merge editable mappings (#7026)
  store correct artifacts for wkorg nightly (#7060)
  Team edit modal (#7043)
  Fix voxel offset in chunk name for unsharded neuroglancer precomputed datasets (#7062)
  Fix offset when loading non-aligned buckets for zarr/n5/precomputed (#7058)
  fixes wallTimes query for workflow reports (#7059)
  Handle Remote Dataset Edge Cases: compressed content-encoding, float voxel size (#7041)
  Release 23.05.2 (#7056)
  ...
hotzenklotz added a commit that referenced this pull request May 17, 2023
@hotzenklotz
Merge branch 'master' of github.com:scalableminds/webknossos into rem…
b5a788f 
…ove_wkconnect

* 'master' of github.com:scalableminds/webknossos: (33 commits)
  Fix bug in fallback rendering when layer has missing mags (#7082)
  Fix duplicate on volumes with no fallback layer (#7085)
  fix scrolling in organization switcher in terms-of-services modal (#7083)
  Auto-Select via SAM (#7051)
  Build STL in chunks when exporting them (#7074)
  Improve performance for large (oversegmentation) meshes (#7077)
  Fix display of used storage in power plan (#7057)
  Fix user limits for invites (#7078)
  adds fileSize to voxelytics workflow list (#7071)
  Improve error logging for bucket requests (#7053)
  Fix zarr streaming datasource-properties.json generation for non-wkw/zarr datasets (#7065)
  Min length for layer names is one (#7064)
  Include voxelytics workflow name in tab title (#7070)
  Fix local to global layer index look up (#7066)
  Merge editable mappings (#7026)
  store correct artifacts for wkorg nightly (#7060)
  Team edit modal (#7043)
  Fix voxel offset in chunk name for unsharded neuroglancer precomputed datasets (#7062)
  Fix offset when loading non-aligned buckets for zarr/n5/precomputed (#7058)
  fixes wallTimes query for workflow reports (#7059)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philippotto philippotto philippotto left review comments

@frcroth frcroth frcroth approved these changes

Assignees

@fm3 fm3

Labels
backend enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clarify Merge Annotation Modal Support merging of proofreading annotations
3 participants
@fm3 @philippotto @frcroth