-
Notifications
You must be signed in to change notification settings - Fork 6
EVA-3818: Fixes for biosample accession, xlsx conversion, and file filtering #86
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
Conversation
|
|
tcezard
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.
Change looks good but we should update the other xlsx files.
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.
I've updated the tooltips for imputation and phasing to match the message in the help section.
We also need to update the other xlsx files in the repo to keep them in sync.
I was wondering if we should update the the minimum required version the realised that it is still set to 1.1.6
I think that should be updated to 2.0 at least
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.
Thanks, I've updated the other xlsx files (though not the tooltips...) and set the min required version to be 2.0.0.
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 there should be a test that checks the different xlsx file in the repo have the same version so we know which one to update. We don't need to do it now though.
In addition to the code changes, updated the metadata template as follows:
Enter '1' if this was an imputation analysis, otherwise leave blankTrue, but I was worried listing them would be too confusing'1'or1(with or without single quotes), so I removed the extra note emphasizing thatv2.0.1