search

neuropycon / graphpype

Neuropycon package of functions for electrophysiology analysis, can be used from ephypype and nipype
https://neuropycon.github.io/graphpype/
BSD 3-Clause "New" or "Revised" License
21 stars 11 forks source link

Autopep8 #11

Closed davidmeunier79 closed 6 years ago

davidmeunier79 commented 6 years ago

I have been using autopep8 on the previous branch I used to format code according to PEP8 requirements. Also, added my e-mail on local config for commits, seems to have changed the way the commits are understood.

codecov[bot] commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@f19fdcd). Click here to learn what that means. The diff coverage is 14.65%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #11   +/-   ##
=========================================
  Coverage          ?   20.44%           
=========================================
  Files             ?       18           
  Lines             ?     2313           
  Branches          ?        0           
=========================================
  Hits              ?      473           
  Misses            ?     1840           
  Partials          ?        0
Impacted Files Coverage Δ
graphpype/interfaces/__init__.py 75% <ø> (ø)
graphpype/nodes/correl_mat.py 24.4% <ø> (ø)
graphpype/pipelines/__init__.py 100% <ø> (ø)
graphpype/utils_net.py 13.7% <10.63%> (ø)
graphpype/tests/test_nii_to_conmat.py 100% <100%> (ø)
graphpype/utils_cor.py 14.84% <11.8%> (ø)
graphpype/utils_plot.py 8.87% <12.72%> (ø)
graphpype/utils_dtype_coord.py 27.77% <31.57%> (ø)
graphpype/utils.py 26.19% <33.33%> (ø)
graphpype/nodes/modularity.py 38.98% <35.48%> (ø)
... and 4 more

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 f19fdcd...90df7e9. Read the comment docs.

jasmainak commented 6 years ago

added my e-mail on local config for commits, seems to have changed the way the commits are understood.

also add the email on github :)

jasmainak commented 6 years ago

@davidmeunier79 please merge when you are happy. We don't know the code well enough to review a 9000 line diff :)

also, try to avoid merge commits if you can ... you should never pull/merge in a professional workflow. Only rebase

jasmainak commented 6 years ago

I meant non-fast forward pulls. Fast-forward pulls are fine ...

davidmeunier79 commented 6 years ago

Ok great, if I remove the dependancy on nipype plugins if passes the test OK.

@jasmainak I am not sure I understand your last point? Indeed I have tested the whole afternoon why it was crashing with nipype plugins, you mean I should have done a new PR every time?

jasmainak commented 6 years ago

umm ... I meant the following:

Let's say you created a branch dev from master and Etienne made another branch fancy-feature from master. You both work on your branches. But Etienne makes a pull request first and it is merged into master. Now, you have conflicts with master. What should you do?

option 1: fetch the latest master and merge it into dev option 2: fetch the latest master but rebase dev on top of master

Option 2 is what you should do. But from what I see, you have been doing option 1

jasmainak commented 6 years ago

The difference between the two options is that option 2 results in a clean commit history. The commits in master should be organized in blocks of pull requests. Something like this:

o-o-o-o-x-x-x-x-x-x-x-y-y-y-y-y-y

where 'o', 'x', and 'y' are the commits of different pull requests. If you follow option 1, you will have something like this:

o-x-o-y-y-x-o-y-o-y-x-x-x-y-x-x-x-y