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

add support for tanglegram #15

Open
wants to merge 11 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@taylorreiter
Copy link
Contributor

commented May 10, 2019

No description provided.

taylorreiter added some commits May 10, 2019

Show resolved Hide resolved KEGGDecoder/KEGG_decoder.py Outdated
Show resolved Hide resolved KEGGDecoder/KEGG_decoder.py Outdated

taylorreiter and others added some commits May 10, 2019

Update KEGGDecoder/KEGG_decoder.py
Co-Authored-By: Luiz Irber <luizirber@users.noreply.github.com>
Update KEGGDecoder/KEGG_decoder.py
Co-Authored-By: Luiz Irber <luizirber@users.noreply.github.com>

@taylorreiter taylorreiter referenced this pull request May 10, 2019

Closed

Push to pypi? #4

taylorreiter added some commits May 11, 2019

@codecov-io

This comment has been minimized.

Copy link

commented May 11, 2019

Codecov Report

Merging #15 into master will decrease coverage by 1.49%.
The diff coverage is 62.08%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #15     +/-   ##
=========================================
- Coverage   54.99%   53.49%   -1.5%     
=========================================
  Files           6        6             
  Lines        1151     1187     +36     
  Branches      386      389      +3     
=========================================
+ Hits          633      635      +2     
- Misses        365      398     +33     
- Partials      153      154      +1
Impacted Files Coverage Δ
KEGGDecoder/KEGG_decoder.py 63.5% <62.08%> (-2.37%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86b2e37...8d65d87. Read the comment docs.

@taylorreiter

This comment has been minimized.

Copy link
Contributor Author

commented May 11, 2019

@luizirber is my strategy here ok? The tab vs. spaces was a bit of a nightmare...I reformatted all of the tabs to spaces in the whole document.

@bjtully...that also means I switched the tabs to spaces, which you may not feel comfortable with in the long run. It looks quite a bit different, but I think its a more standard formatting. However, we can come up with another strategy to put this into the code if you'd like!

@bjtully

This comment has been minimized.

Copy link
Owner

commented May 11, 2019

@taylorreiter I have very strong opinions about the changes from tabs to spaces...

@luizirber

This comment has been minimized.

Copy link
Contributor

commented May 11, 2019

I'll have to disagree with both of you =]

I prefer spaces to tab and think choosing tabs make it harder to have contributions, but if these are the conventions adopted in this repo then we need to adapt to it.

@taylorreiter let's say you receive a PR that adds a feature, but changes a thousand lines of (unrelated) code along the way. How would you review it? How long does it take to figure out if a line changed because of formatting, or if it is something that changes the behavior of the code?

@taylorreiter

This comment has been minimized.

Copy link
Contributor Author

commented May 11, 2019

I'll switch it back! Now that I know I can get all checks to pass, I'll write the same thing with tabs only and submit a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.