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

Define required NWB fields in the context models #51

Open
luiztauffer opened this issue Nov 6, 2023 · 11 comments
Open

Define required NWB fields in the context models #51

luiztauffer opened this issue Nov 6, 2023 · 11 comments

Comments

@luiztauffer
Copy link
Collaborator

@magland how should we define, in the Processor context models, NWB specific fields?

Here's one idea, using json_schema_extra. The nwb_field key would accept strings in a format that would allow the frontend to identify and search for the available options of that type inside the chosen NWB file. The names are tentative:

from pydantic import BaseModel, Field

class Mymodel(BaseModel):
    electrical_series_path: str = Field(default=None, json_schema_extra={"nwb_field": "ElectricalSeries.path"})
    electrodes_location_x: str = Field(default="rel_x", json_schema_extra={"nwb_field": "Electrodes.column_name"})
    electrodes_location_y: str = Field(default="rel_y", json_schema_extra={"nwb_field": "Electrodes.column_name"})
    electrodes_location_z: str = Field(default="rel_z", json_schema_extra={"nwb_field": "Electrodes.column_name"})

d = Mymodel()
d.model_json_schema()

resulting schema:

{
    'properties': {
        'electrical_series_path': {
            'default': None,
            'nwb_field': 'ElectricalSeries.path',
            'title': 'Electrical Series Path',
            'type': 'string'
        },
        'electrodes_location_x': {
            'default': 'rel_x',
            'nwb_field': 'Electrodes.column_name',
            'title': 'Electrodes Location X',
            'type': 'string'
        },
        'electrodes_location_y': {
            'default': 'rel_y',
            'nwb_field': 'Electrodes.column_name',
            'title': 'Electrodes Location Y',
            'type': 'string'
        },
        'electrodes_location_z': {
            'default': 'rel_z',
            'nwb_field': 'Electrodes.column_name',
            'title': 'Electrodes Location Z',
            'type': 'string'
        }
    },
    'title': 'Mymodel',
    'type': 'object'
}
@luiztauffer
Copy link
Collaborator Author

@magland we could start with a fixed list of allowed nwb_file values such as ElectricalSeries.path and Electrodes.column_name (or similar names) which the central service would know how to map any given NWB file to find options for the requested field.
I'm curious, where is the mapping for electrical series path happening in the current sorting examples? Is it done by the frontend code or by a request to the rest API?

@magland
Copy link
Collaborator

magland commented Nov 6, 2023

I'm curious, where is the mapping for electrical series path happening in the current sorting examples? Is it done by the frontend code or by a request to the rest API?

The frontend reads from the remote nwb/hdf5 file directly.

I like your idea, but it will be challenging to come up with the right syntax. Depending on the desired criteria, this could get complicated quickly.

A related note. When running spike sorting in batch, right now the GUI only reads the options from the first selected file, as it would be potentially expensive to read from all of the selected files. And then what if there were inconsistencies.... how to present that in the input form?

@luiztauffer
Copy link
Collaborator Author

luiztauffer commented Nov 6, 2023

When running spike sorting in batch,...

I think it's fine for any batch processing right now to presume consistent NWB files in terms of these fields

but it will be challenging to come up with the right syntax.

Yeah I agree, but as a first approach, we can have it simple, just a couple of pre-defined types would be allowed, and we go building more case-by-case as the needs of new Processors appear

@magland
Copy link
Collaborator

magland commented Nov 6, 2023

Okay. Do you think we should just forward everything in json_schema_extra directly to the spec.json? Or do we want to have a predefined allowed fields? Right now we have 'secret' and 'options'. I'm just thinking about how we eventually might want some sort of directive that is handled on the Python side but does not get forwarded to the schema.

@luiztauffer
Copy link
Collaborator Author

since we'll be implementing explicitly what each of these extra props do on the frontend, I think it would be better to have a pre-defined list

@magland
Copy link
Collaborator

magland commented Nov 7, 2023

What do you think of the following

class Mymodel(BaseModel):
    electrical_series_path: str = Field(default=None, json_schema_extra={"nwb_field": {'type': 'object_path', 'neurodata_type': 'ElectricalSeries', 'parent_path': 'acquisition'})

@magland
Copy link
Collaborator

magland commented Nov 9, 2023

@luiztauffer Based on our discussion, what do you think about this for now:

class Mymodel(BaseModel):
    electrical_series_path: str = Field(default='', json_schema_extra={"tags": ['nwb_electrical_series_path'])

@magland
Copy link
Collaborator

magland commented Nov 9, 2023

Actually I'm going to revise that

class Mymodel(BaseModel):
    electrical_series_path: str = Field(default='', json_schema_extra={'nwb_field': {'type': 'electrical_series_path', 'for': 'input'}})

@luiztauffer
Copy link
Collaborator Author

what would for work for in here?

@magland
Copy link
Collaborator

magland commented Nov 9, 2023

what would for work for in here?

It specifies that we're selecting an electrical series in the input file 'input' (a processor may in principle have multiple input files)

@luiztauffer
Copy link
Collaborator Author

makes sense to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants