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

Add list value support for missing fields #1789

Closed
sellth opened this issue Sep 12, 2023 · 6 comments
Closed

Add list value support for missing fields #1789

sellth opened this issue Sep 12, 2023 · 6 comments
Assignees
Labels
app: samplesheets Issue in the samplesheets app feature Requested feature or enhancement
Milestone

Comments

@sellth
Copy link
Contributor

sellth commented Sep 12, 2023

Describe the Bug

Inputting a semicolon separated list in any Characteristics, Comment, Parameter, or Factor field results in AltamISA creating a list value for that object. In case of Characteristics and Parameters these lists are handled by SODAR without any problems. Lists in special protocol parameter Performer fields however result in raw Python list notation being rendered (i.e. ['Person 1', 'Person 2']). Entering semicolons in Comment fields results in an exception and a no longer usable ISA-tab.

How to Reproduce

  1. Create a new SODAR project.
  2. Edit samplesheets.
  3. Add semicolon-separated values in Comment and Performer fields.

Expected Behavior

Lists should be rendered in a similar way to Characteristics and Parameter fields. As Comments and Performer fields cannot be differentiated from "normal" characteristics and parameters in the SODAR samplesheet editor, this case needs to be handled. Putting multiple performers for a Process is actually a useful option when more than one person should be credited.

Screenshots

grafik

grafik

@sellth sellth added the bug Something isn't working label Sep 12, 2023
@mikkonie
Copy link
Contributor

It's not broken, it has never been implemented. I haven't been told this is a feature in altamISA nowadays.

If all characteristics and factor values are to be included, this will be a major change. We will have to see about prioritization.

@mikkonie mikkonie removed the bug Something isn't working label Sep 12, 2023
@mikkonie mikkonie changed the title List rendering broken for Comment and Performer fields Add list value support for missing fields Sep 12, 2023
@mikkonie mikkonie added feature Requested feature or enhancement tbd Comments wanted, spec/schedule/prioritization to be decided, etc. app: samplesheets Issue in the samplesheets app labels Sep 12, 2023
@sellth
Copy link
Contributor Author

sellth commented Sep 12, 2023

Actually, the AltamISA part is wrong. I did not check before, but it seems like AltamISA is not processing these values into lists. So the issue as of now is: adding semicolons using SODAR's own samplesheet editor results in weird code being shown or an uncaught exception during rendering.

@mikkonie
Copy link
Contributor

It's not really weird code if the value is unsupported :) The crash should be avoided though. Can you (privately) send me a copy of the samplesheets where such a crash occurs?

@sellth
Copy link
Contributor Author

sellth commented Jan 30, 2024

Altamisa feature request: bihealth/altamisa#140

@mikkonie mikkonie removed the tbd Comments wanted, spec/schedule/prioritization to be decided, etc. label Oct 30, 2024
@mikkonie mikkonie self-assigned this Oct 30, 2024
@mikkonie mikkonie added this to the v1.0.0 milestone Oct 30, 2024
@mikkonie
Copy link
Contributor

Doing this as a part of #2033.

@mikkonie
Copy link
Contributor

Done. If I overlooked something or you run into bugs, please open new ticket(s) about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app: samplesheets Issue in the samplesheets app feature Requested feature or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants