-
Notifications
You must be signed in to change notification settings - Fork 0
SN Update column mappings #21
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
base: master
Are you sure you want to change the base?
Conversation
wfeng19
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides a couple of typos, everything looks good to me!
| "DuplexSeq_ExternalQualityMetric": "duplexseq_external_quality_metric", | ||
| "DSA_ExternalQualityMetric": "dsa_external_quality_metric" | ||
| }, | ||
| "column_mappings": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dominik provided recommendations for new descriptions for a couple of the items on this Google Sheets. Many of them seem more like they are describing the importance of the metric and not the metric itself, so I'm not sure how many we want to use, but some could be helpful to add!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I would use the updated version of Poetry to generate the poetry.lock file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you regenerated this file using an older version of Poetry. The version in the MAKEFILE is 1.8.4 and ideally that should be used to generate the poetry.lock
| "DSA_ExternalQualityMetric": "dsa_external_quality_metric" | ||
| }, | ||
| "column_mappings": { | ||
| "external_quality_metric": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a breaking change or has this functionality not been used before? Will older spreadsheets still work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functionality hasn't been used before. This was something David has made as a template, but that we haven't actually taken in any submissions with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Sounds good.
pyproject.toml
Outdated
| [tool.poetry] | ||
| name = "smaht-submitr" | ||
| version = "1.6.3" | ||
| version = "1.6.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a minor version change is appropriate? (I assume there is no breaking change)
custom_column_mappingsto include values for both DuplexSeq_ExternalQualityMetrics and DSA_ExternalQualityMetrics