Add transcript_support_level (TSL) field

openvax / pyensembl

Python interface to access reference genome features (such as genes, transcripts, and exons) from Ensembl

Apache License 2.0

374 stars 65 forks source link

Add transcript_support_level (TSL) field #212

Closed vladsavelyev closed 6 years ago

vladsavelyev commented 6 years ago

GRCh38 database provides a field called transcript_support_level (TSL) for transcript features. This PR adds reading of it as an optional field, and adds a new support_level field for Transcript object.

iskandr commented 6 years ago

Thanks @vladsaveliev -- this looks like a good change.

I noticed that the Travis build when it tries to use our private netmhcbundle package. This might be a permissions problem, I'll look into it.

In the mean time, do you mind adding a unit test for this property?

vladsavelyev commented 6 years ago

Thanks @iskandr!

Just added a test for this.

iskandr commented 6 years ago

Failing on Travis due to this issue: https://github.com/travis-ci/travis-ci/issues/1946 (encrypted values not available on PRs from forks)

iskandr commented 6 years ago

Only solution I've seen so far seems pretty complicated: https://blog.algolia.com/travis-encrypted-variables-external-contributions/

iskandr commented 6 years ago

Hey @vladsaveliev -- once I merge https://github.com/openvax/pyensembl/pull/213, can you rebase your branch? That should enable unit tests to run on Travis for your code.

iskandr commented 6 years ago

OK @vladsaveliev -- if you rebase then Travis should work for your PR.

vladsavelyev commented 6 years ago

Rebased!

iskandr commented 6 years ago

Whew, almost there. Found two failing serialization tests:

test.test_serialization.test_json_ensembl_transcript
test.test_serialization.test_pickle_ensembl_transcript

I'm guessing they're missing the TSL?

vladsavelyev commented 6 years ago

Right, because I wasn't adding support_level field into the state_dict returned by to_dict(), that is used by Serializable to serialize to json. Fixed now.

coveralls commented 6 years ago

Coverage increased (+0.3%) to 80.363% when pulling 7bf09bf8f676744d951e6f9a5163acde204f0538 on vladsaveliev:master into 8ffde7ae73ffe93d42317ce95091f7019888cf11 on openvax:master.

coveralls commented 6 years ago

Coverage increased (+0.2%) to 80.298% when pulling c977ed6b462d624ce3c429c2d288166ff5d2b782 on vladsaveliev:master into 8ffde7ae73ffe93d42317ce95091f7019888cf11 on openvax:master.

iskandr commented 6 years ago

Ok, looks great. Can you bump the version number in init.py? Then I'll merge the PR and a successful Travis run on master will deploy to PyPI.

vladsavelyev commented 6 years ago

Done!

iskandr commented 6 years ago

Merged! Thanks for the PR!

Also, feel free to leave questions on the vaxrank and docker pipeline repos if you encounter anything confusing there.

vladsavelyev commented 6 years ago

By the way, thank you for all the tools you guys do in openvax. It's remarkably high quality.

iskandr commented 6 years ago

No problem -- our big weakness has so far been documenting and publicizing what we're building. Feel free to ask about anything that's unclear or nudge us to document things better.