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

Remove redundant symbolic alt to so term hash #1643

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

nakib103
Copy link
Contributor

@nakib103 nakib103 commented Mar 21, 2024

@nakib103 nakib103 changed the base branch from postreleasefix/112 to main May 1, 2024 09:46
@nakib103 nakib103 marked this pull request as draft May 1, 2024 09:46
@nakib103 nakib103 changed the base branch from main to postreleasefix/113 August 5, 2024 08:23
@nakib103 nakib103 changed the base branch from postreleasefix/113 to main August 20, 2024 13:51
@nakib103 nakib103 added the e114 label Aug 21, 2024
@nakib103
Copy link
Contributor Author

@nuno-agostinho,
The unit test on one before the last take accounts the change in ensembl-variation and passed. You can ignore the unit test failure in the last commit.

@nakib103 nakib103 marked this pull request as ready for review August 21, 2024 13:15
Copy link
Contributor

@nuno-agostinho nuno-agostinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as expressed in Ensembl/ensembl-variation#1075 (review).

Otherwise, I just have small suggestions.

our @EXPORT_OK = qw(
get_SO_term
&check_format
&_valid_region_regex
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to export _valid_region_regex, it was only being used within check_format for ensembl-variation (confirm?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is used in -

modules/Bio/EnsEMBL/VEP/Parser.pm Outdated Show resolved Hide resolved
Co-authored-by: Nuno Agostinho <nunodanielagostinho@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants