Make interface display names more human readable

[rly]

Temporary solution for #588 until next NeuroConv release with the display names as class variables for each interface. cc @bendichter

We don't need to merge this if the NeuroConv release schedule is relatively fast.

Result:

After adding an interface, the key is still the name of the interface:

I wasn't sure what the downstream effects of changing that are so I left that as is.

[CodyCBakerPhD]

Temporary solution for #588 until next NeuroConv release with the display names as class variables for each interface

We don't need a release of NeuroConv to make it work; NWB GUIDE packages the main branch

[CodyCBakerPhD]

I'm not sure what the "(All Data)" part adds; what are you trying to say with that?

[CodyCBakerPhD]

I wouldn't do this on the TypeScript side; I'd modify the values returned by the NeuroConv query on the backend since that's a lot more customizable: https://github.com/NeurodataWithoutBorders/nwb-guide/blob/interface_display_name/pyflask/manageNeuroconv/manage_neuroconv.py#L165-L170

You can also set it up there to retrieve the 'display_name' if set as an attribute on the data interface classes

Did you want to work on that @rly?

[garrettmflynn]

Updated to align with NeuroConv and the rest of the GUIDE repo, as displayName should've just been used in GuidedStructure.js as the key value, rather than replacing key in the Search and List components.

Note that several converters and interfaces still need to be named in NeuroConv:

[CodyCBakerPhD]

LGTM except for failing tests

Fine with getting this through as is and adding more display names on NeuroConv over time

[garrettmflynn]

Thanks for flagging that. Tests are passing now!

[CodyCBakerPhD]

@garrettmflynn Is this configured to use catalystneuro/neuroconv#734 (with or without suffix functionality, which can be in a follow-up)? Just want to make sure it's compatible with that so we don't unintentionally break something once that is merged

[garrettmflynn]

Just updated so that the info attribute is used preferentially to __doc__. Also enabled searching by suffix.

Holding off on the use of associated_suffixes in the Data Formats page for a follow-up. That is the desired target location for both the informational and programmatic use of this attribute, right?

[CodyCBakerPhD]

Just updated so that the info attribute is used preferentially to doc.

Yeah I thought about just defering to the doc but the downside is our class docstrings have a lot of .rst and sphinx formatting for the autogeneration in the API section. So the info is intended to be a shorter, text-only version of that.

That is the desired target location for both the informational and programmatic use of this attribute, right?

The NeuroConv PR currently has an attribute on every class called associated_suffixes which is a tuple of strings. If that PR is merged as is what would be the target location for retrieving the information

[garrettmflynn]

Agreed. I'm parsing the docstring, but it's a bit messy to do that.

The NeuroConv PR currently has an attribute on every class called associated_suffixes which is a tuple of strings. If that PR is merged as is what would be the target location for retrieving the information

Ah I accidentally referred to the Source Data page as the Data Formats page above.

To reiterate, I've added the ability to search by associated_suffixes on the Data Formats page.

The discussion on the related NeuroConv PR has been more related to the Source Data page, though, and filtering the file selector (as much as we can) on that page by adding the associated_suffixes to the accept attribute. We'll also want to inject this information somewhere as a documentation string, either in the schema description (displayed below the input) or on the gray selection box itself.

[CodyCBakerPhD]

@garrettmflynn an e2e conflict on this one

[CodyCBakerPhD]

@garrettmflynn catalystneuro/neuroconv#734 is mostly ready to go now; shall we do a pass and see if this is ready for review?

[CodyCBakerPhD]

With that PR you can use a new function to get what you need

get_format_summaries imported by from neuroconv import get_format_summaries

[garrettmflynn]

Updated! However, for using get_format_summaries to make much sense, I've declared the results as a global variable. Otherwise, I was grabbing the values on an as-needed basis directly from the interface.

[CodyCBakerPhD]

@garrettmflynn Still a merge conflict with the e2e tests

[garrettmflynn]

Ah missed the merge conflict. Was just trying to nail the test failure down

[garrettmflynn]

@CodyCBakerPhD All of these tests are now going to fail because of the reliance on the NeuroConv branch. Previously, I was just using fallbacks if things didn't exist—but that new function can't be handled the same way.

How do you want to mediate that in the meantime?

[CodyCBakerPhD]

Either update the environment files point to the relevant branch or wait until it gets merged (and only use test this branch locally using the right upstream) - only 2 options for us

The former would take more effort since we'd have to change it back after the upstream merge

[garrettmflynn]

I'm fine with the latter if you're fine merging with tests failing 😅

[CodyCBakerPhD]

Oh no, not what I meant lol 😆 this should either not be merged until upstream is, or pin to NeuroConv dev branch it's compatible with and immediately update the pins after that merge

[garrettmflynn]

Ah gotcha lol

[CodyCBakerPhD]

Yep, this looks great now

Just need a better search functionality in a follow-up to find things like "phy" or "sorted" (as in, "spike sorted" currently only finds "sorting")