Skip to content

Conversation

@apriltuesday
Copy link
Contributor

@apriltuesday apriltuesday commented May 27, 2025

In addition to the code changes, updated the metadata template as follows:

  • Help text for imputation/phasing now reads Enter '1' if this was an imputation analysis, otherwise leave blank
    • Technically we actually accept multiple values as True, but I was worried listing them would be too confusing
    • We now accept either '1' or 1 (with or without single quotes), so I removed the extra note emphasizing that
  • Help text for BioSample accession was already correct (no mention of ENA or EGA), corrected a couple typos
  • Bumped version to v2.0.1

@apriltuesday apriltuesday marked this pull request as ready for review May 28, 2025 11:57
@apriltuesday apriltuesday self-assigned this May 28, 2025
@apriltuesday apriltuesday requested review from nitin-ebi and tcezard May 28, 2025 11:57
@apriltuesday
Copy link
Contributor Author

apriltuesday commented May 28, 2025

Failing tests are due to an issue in BioSamples, they are looking into it. Now fixed.

Copy link
Member

@tcezard tcezard left a 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.

Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

@tcezard tcezard May 30, 2025

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.

@apriltuesday apriltuesday merged commit aaff951 into EBIvariation:main Jun 2, 2025
1 check passed
@apriltuesday apriltuesday deleted the EVA-3818 branch June 2, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants