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

Feature: adds display of tag descriptions and regexp searching for tags #528

Merged
merged 11 commits into from
Mar 18, 2024

Conversation

fcollman
Copy link
Contributor

tag descriptions were not displayed to the user anywhere, so i've added those.

Also, for long tag lists there wasn't a good way to search through the tags.

This implementation assumes that if there were no results from the regexp search in the segment labels that the user might also want to search through the tags and tag descriptions and limit that list instead.

@jbms
Copy link
Collaborator

jbms commented Feb 25, 2024

I am worried that it may be problematic for the behavior to depend on whether there are any label matches. To avoid that, there could be a dedicated syntax for searching tag names/descriptions, e.g. "#/". On the other hand an additional syntax adds complexity for users, and I suppose it may be the case in practice that labels are not used if tags are used.

Currently the query produces a list of segments that match, and the set of tags displayed is exactly the set of tags that match the query (or all tags if there is no query).

An option to filter the list of tags would break this invariant, because a segment could have one tag that matches the tag regexp and another tag that doesn't match the regexp. If we filter the list of tags, it would be natural to also filter the segments to those that have a matching tag. But then users might be interested in which other tags the matching segments have, and there would be no way to see that.

@fcollman
Copy link
Contributor Author

I think the issue of it matching one one tag but not another is not an issue. The user simply presses the + button on the tag they were looking for.. the regexp goes away and the tag is added and then you have the behavior you were hoping for where you see the summary of other tags that remain in the list of segments (along with the #).

I'm not sure there are a lot of use cases where there are tags and labels that have overlapping search spaces.

I came up with one, where you have a hierarchical style segmentation, say of brain areas, where you have low level segment IDs that merge together into higher ones. Each segment at each level has a name, but perhaps you tag all the lower level segments with the names/abbreviations of the higher level segments. In this way, you could add all the segments that are underneath one node, or see when you filter to a lower level segment, what are all the higher level segments in the hierarchy that are associated with that lower level.

In that use case I think this interface actually works pretty well. As you search for things it will default to searching for the labels themselves and the individual elements of the hierarchy, and not really be searching through the tags that have overlapping character strings.

I think you could have the syntax that you suggested in cases where you really want to search tags.

Zooming out, if this isn't the solution I feel like one has to have a different kind of solution to deal with situations where you have more tags than easily fit in the small window. Scrolling and hunting for the tag you want is not a great UI.

@jbms
Copy link
Collaborator

jbms commented Feb 26, 2024

I agree that filtering by tag is important, and the basic approach you have here seems reasonable.

A few fixes are needed to make this work if there is no label property.

@fcollman
Copy link
Contributor Author

can you point me to an example with a valid segment_properties files with no labels so I can figure out how to modify this implementation to work correctly for that case?

@jbms
Copy link
Collaborator

jbms commented Feb 26, 2024

can you point me to an example with a valid segment_properties files with no labels so I can figure out how to modify this implementation to work correctly for that case?

https://h01-dot-neuroglancer-demo.appspot.com/#!gs://h01-release/assets/neuroglancer_states/20210601/gallery/00_l2_interneurons.json

@fcollman
Copy link
Contributor Author

okay i've given it a shot.. i'm not certain if i've covered all the cases though. Please give it a second look.. @jbms

filterIndices((index) => values[index].match(regexp) !== null);
}
if (prefix !== undefined) {
console.log("prefix", prefix);
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove console.log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed!

const filterByTagDescriptions = (regexp: RegExp) => {
const tagDescriptions = db!.tags!.tagDescriptions!;
const tags = db!.tags!.tags!;
const matchingTagDescriptions = tagDescriptions
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would probably be more efficient to just iterate over the tags with a for loop --- then we wouldn't need to create all of these temporary arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think i've addressed this.. it's failing tests for unrelated reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see #544 for python linting error fix

@jbms jbms merged commit 7d75cf3 into google:master Mar 18, 2024
13 of 19 checks passed
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.

None yet

2 participants