cidgoh / DataHarmonizer

A standardized browser-based spreadsheet editor and validator that can be run offline and locally, and which includes templates for SARS-CoV-2 and Monkeypox sampling data. This project, created by the Centre for Infectious Disease Genomics and One Health (CIDGOH), at Simon Fraser University, is now an open-source collaboration with contributions from the National Microbiome Data Collaborative (NMDC), the LinkML development team, and others.
MIT License
90 stars 23 forks source link

Pulldown/select menus should not be created for any slot/column whose range isn't some enum #320

Closed turbomam closed 2 years ago

turbomam commented 2 years ago

At some point I unilaterally assumed that a pulldown menu would only be generated if the range was some enum. Therefore, I could use string/multivalued to indicate that multiple values could be entered into the cell, separated by some delimiter.

Then subsequent code (like submissions_as_studies.py. ) would break the multiple entries into a structured list.

Unfortunately DH is generating empty multi-select menus for my string-range multivalued slots/columns.

See also https://github.com/microbiomedata/sheets_and_friends/issues/134

and (private) NMDC Slack thread https://nmdc-group.slack.com/archives/C03LVV2ETS7/p1655989526260609

ddooley commented 2 years ago

Ok, looks like that will be easy to solve. Indeed "multivalued" on its own shouldn't trigger multi-select menu display.

turbomam commented 2 years ago

Great. Should I hold off on my interim fix? I'll try to get a sense of urgency from Montana.

ddooley commented 2 years ago

yes hold off, this should be a quick fix. But can you list the yaml slot definition for the field where this is happening?

ddooley commented 2 years ago

So far I'm seeing that only when a slot's range is matched to an enum will the picklist control be activated on a field.

mslarae13 commented 2 years ago

Do you need a list the fields I find broken?

Urgency isn't until next sprint / post review

turbomam commented 2 years ago

thanks @mslarae13. The problematic fields are already listed here

turbomam commented 2 years ago

@ddooley coud you please visit https://microbiomedata.github.io/sheets_and_friends/main?template=nmdc_submission_schema/soil_emsl_jgi_mg and start with this sample data file

All of the columns listed here have the problematic behavior, take a string range (not an enum) and are multivalued

turbomam commented 2 years ago

Here's a relevant snippet:

      air_temp_regm:
        name: air_temp_regm
        annotations:
          expected_value:
            tag: expected_value
            value: temperature value;treatment interval and duration
          preferred_unit:
            tag: preferred_unit
            value: meter
          occurrence:
            tag: occurrence
            value: m
        description: Information about treatment involving an exposure to varying
          temperatures; should include the temperature, treatment regimen including
          how many times the treatment was repeated, how long each treatment lasted,
          and the start and end time of the entire treatment; can include different
          temperature regimens
        title: air temperature regimen
        examples:
        - value: 25 degree Celsius;R2/2018-05-11T14:30/2018-05-11T19:30/P1H30M
        from_schema: http://w3id.org/mixs/terms
        aliases:
        - air temperature regimen
        rank: 16
        is_a: core field
        string_serialization: '{float} {unit};{Rn/start_time/end_time/duration}'
        slot_uri: MIXS:0000551
        multivalued: true
        owner: MIMS
        slot_group: MIxS (modified)
        range: string
        recommended: true
pkalita-lbl commented 2 years ago

I think the right fix would be to move this check on the flatVocabulary field up to the top level check to enter that block in the first place (i.e. only enter that block if the field is multivalued and has options to choose from). I'm happy to make that change if that sounds like the correct desired behavior.

ddooley commented 2 years ago

This should be fixed in DH v. 1.1.0 on master branch. Line 942 of source/dataharmonizer/index.js needed adjustment. Right n the money Patrick!

ddooley commented 2 years ago

I think this can be closed now?