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

feat(datasource/graphene) added timestamp tool to graphene, timestamp property to SegmentationUserLayer #613

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrisj
Copy link
Contributor

@chrisj chrisj commented Jul 15, 2024

replaces #587

This is also designed to support our future cave annotation layers being able to synchronize/control with the segmentation timestamp.

Here is part of that implementation seung-lab@4d41b32

…ies a new timestamp property on SegmentationUserLayer
@chrisj
Copy link
Contributor Author

chrisj commented Jul 15, 2024

Also, can you explain what you use to organize imports? Is it using the eslintrc.yml file? The default vs code sorter wants to sort lowercase above all uppercase imports and doesn't have the same behavior with package and css imports.

@jbms
Copy link
Collaborator

jbms commented Oct 31, 2024

You can use npm run format:fix and npm run lint:fix --- it is indeed eslint.

@jbms
Copy link
Collaborator

jbms commented Oct 31, 2024

Can you explain the logic of timestampOwner? It looks like the purpose is to link the timestamp across layers, but why canSetTimestamp?

@chrisj
Copy link
Contributor Author

chrisj commented Nov 1, 2024

I'm currently using timestamp owner to do a self-lock when our graphene layer has an active edit tool (owner = self + no timestamp). canSetTimestamp says you to change the timestamp when there is no owners or the only owner is the one checking. Alternatively, I could make a setter function for timestamp that does the check and also sets owner.

The cave annotation datasource has more use of timestamp owner. I plan to put up a PR for if this PR is approved.
seung-lab@5dd2b7d

The cave annotation layers have a timestamp associated with it, when it gets linked to a segmentation layer, it sets the segmentation layer to the same timestamp if possible. Multiple annotation layers with the same timestamp can be linked to the same segmentation layer. Each one gets added as an owner. Only once all of them are unlinked is the segmentation timestamp available to be changed (or if one remaining owner wants to change the timestamp it has ownership of).

There's more work to be done with the cave annotation implementation. At some point I think we would want the option to change the timestamp of all linked annotation layers together without having to unlink then re-link.

@jbms
Copy link
Collaborator

jbms commented Nov 5, 2024

It occurs to me that it could potentially make sense to treat the timestamp as a special type of dimension that can be either global or local to a single layer.

It would need to differ from the existing dimensions in that it would display as a timestamp and support the time picker UI, and would be excluded from the affine transforms.

Do I understand correctly that when editing the segmentation, the idea is that you disregard the "lock" and temporary switch to the latest segmentation rather than the one synchronized with the annotations?

@chrisj
Copy link
Contributor Author

chrisj commented Nov 11, 2024

I like how that fits in with existing concepts in Neuroglancer. Do you see this as replacing the ownership? The issue I see with that is that it wouldn't support being able to compare segmentations at different timestamps, within the same neuroglancer instance, with both having linked annotation layers with matching timestamps. I confirmed with others that we consider that to be valuable/important. I am all for re-thinking the implementation.

So we want segmentation layers to be able to be linked, timestamp synced with multiple annotation layers. We also want to add a duplicate segmentation source and duplicate annotation layers but set to a different timestamp.

I mentioned before that I eventually want to be able to change the timestamp for all linked+ timestamp locked layers. I think I can already do that with the current implementation by just adding a confirmation dialog.

Do I understand correctly that when editing the segmentation, the idea is that you disregard the "lock" and temporary switch to the latest segmentation rather than the one synchronized with the annotations?

No, I only allow editing tools to be enabled when the segmentation layer is current and a lock is applied to prevent a timestamp change while the editing is active.

@chrisj
Copy link
Contributor Author

chrisj commented Nov 19, 2024

@jbms just bumping this in case you didn't see my last response

@jbms
Copy link
Collaborator

jbms commented Nov 19, 2024

Sorry for the delay in responding.

If we view the timestamp as a dimension, then as I see it, the same mechanisms currently available for comparing at two different values of a given dimension could similarly be employed:

  • Make the segmentation timestamp dimension a layer-local dimension, i.e. ending in ' --- then it would be unlinked from anything else, but this would also mean you couldn't link it to an annotation layer
  • Rename the timestamp dimension of at least one of the segmentations, such that you have two different global timestamp dimensions, e.g. t1 and t2. Then you could freely assign annotation layers to these dimensions as well.
  • Have just a single global timestamp dimension t, but create two tiled layer group viewers, where the t dimension is unlinked. Note that currently, layer group viewers can either have their position linked, or unlinked, to the global position, but the linking applies to all dimensions --- presumably we would want to extend it to allow per-dimension linking, but that is a minor thing. This might be the most convenient way to compare the same segmentation layer at two different times, but it would only allow you to view them side by side, not overlaid.

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