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

Fix Ensembl release 83 #135

Merged
merged 1 commit into from
Feb 22, 2016
Merged

Conversation

iskandr
Copy link
Contributor

@iskandr iskandr commented Feb 22, 2016

  • make sure all the unit tests work with release 83 and/or use the correct release for which they were written
  • added a few unit tests to improve coverage
  • got rid of the top-level objects like ensembl77, ensembl83, &c since it seems like bad design to add another one of those for each Ensembl release

Review on Reviewable

ensembl83 = cached_release(83)
ensembl_grch38 = ensembl83 # most recent for GRCh38

__all__ = [
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know much about __all__ other than some brief Googling; any particular reason you decided to add this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it might be slightly better package etiquette to restrict which things get imported as '*'. I'm not totally sure if it's necessary though.

https://docs.python.org/2/tutorial/modules.html#importing-from-a-package

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@tavinathanson
Copy link
Contributor

LGTM % minor questions

iskandr added a commit that referenced this pull request Feb 22, 2016
@iskandr iskandr merged commit c70d32b into master Feb 22, 2016
@iskandr iskandr deleted the fewer-default-ensembl-release-objects branch July 28, 2018 00:55
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.

None yet

2 participants