search

biocore-ntnu / epic

(DEPRECATED) epic: diffuse domain ChIP-Seq caller based on SICER
http://bioepic.readthedocs.io
MIT License
31 stars 6 forks source link

Add type annotations for all functions #47

Closed DarwinAwardWinner closed 7 years ago

DarwinAwardWinner commented 7 years ago

I've written type annotations for every function in the epic module. This allows you to run mypy and type-check the entire codebase. I also fixed a few minor bugs that were revealed by the type checker.

endrebak commented 7 years ago

Thanks. Never used type annotations in Python before. Look forward to reading PEP484.

There are some typos in your code (typimg), which I guess is why the continuous integration fails?

It seems like type annotations do not work for 2.7 though. Perhaps they can be added in a way compatible with 2.7 like shown here: http://mypy.readthedocs.io/en/latest/python2.html

If you fix that and type annotations does not break anything major, I'll merge the PR.

Endre

On Sunday, November 13, 2016, Ryan C. Thompson notifications@github.com wrote:

I've written type annotations for every function in the epic module. This allows you to run mypy and type-check the entire codebase. I also fixed a

few minor bugs that were revealed by the type checker.

You can view, comment on, or merge this pull request online at:

https://github.com/endrebak/epic/pull/47 Commit Summary

  • Add mypy type annotations
  • Add missing argument to get_island_bins

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/endrebak/epic/pull/47, or mute the thread https://github.com/notifications/unsubscribe-auth/AQ9I0j_FUojhQ5jGJM8-CHgmrPiIxig_ks5q9t5ZgaJpZM4KwpzU .

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.5%) to 55.266% when pulling 0b7b8fac0d8ee01cb3fb0525916b9f256139674a on DarwinAwardWinner:mypy into 952bd54d39e0d8b87290f69bf6f7b061edbc18fa on endrebak:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.5%) to 56.243% when pulling a869f66ba528add32b691cc430294c78288b1aee on DarwinAwardWinner:mypy into 952bd54d39e0d8b87290f69bf6f7b061edbc18fa on endrebak:master.

DarwinAwardWinner commented 7 years ago

OK, I think I've got everything in order. Let me know if you want me to squash some of the fixup commits before you merge.

endrebak commented 7 years ago

Will look closer at this within a few days and before making a new commit to the project. I am a bit swamped in the beginning of the week. Thanks btw!

On Sun, Nov 13, 2016 at 7:17 PM, Ryan C. Thompson notifications@github.com wrote:

OK, I think I've got everything in order. Let me know if you want me to squash some of the fixup commits before you merge.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/endrebak/epic/pull/47#issuecomment-260202280, or mute the thread https://github.com/notifications/unsubscribe-auth/AQ9I0iHOUxM7C2EJJ4rAgpbT8xAB1Jfvks5q91RCgaJpZM4KwpzU .

endrebak commented 7 years ago

Since the tests passed, and the 2.7 version of annotations is really just comments, I merged your PR. Thanks. Added you to the contrib list.