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

Updates to FBC measures documentation #1094

Merged
merged 2 commits into from Jul 20, 2016

Conversation

Projects
None yet
4 participants
@stephanmeesters
Contributor

stephanmeesters commented Jul 4, 2016

Removed some import lines that weren't used. Added a fixed seed for reproducibility.

I was asked to fix PEP8 in this line
mask_lgn[35-rad:35+rad, 42-rad:42+rad, 28-rad:28+rad] = True

However the PEP8 checker I'm using doesn't complain about this line. Should it be changed to this?
mask_lgn[35-rad : 35+rad, 42-rad : 42+rad, 28-rad : 28+rad] = True

@coveralls

This comment has been minimized.

coveralls commented Jul 4, 2016

Coverage Status

Coverage remained the same at 82.908% when pulling f69467e on stephanmeesters:fbc_measures_update_pr into fbad873 on nipy:master.

@codecov-io

This comment has been minimized.

codecov-io commented Jul 4, 2016

Current coverage is 80.84%

Merging #1094 into master will not change coverage

@@             master      #1094   diff @@
==========================================
  Files           200        200          
  Lines         23023      23023          
  Methods           0          0          
  Messages          0          0          
  Branches       2309       2309          
==========================================
  Hits          18614      18614          
  Misses         3933       3933          
  Partials        476        476          

Powered by Codecov. Last updated by fbad873...1b34aec

@arokem

This comment has been minimized.

Member

arokem commented Jul 15, 2016

Hi @stephanmeesters : thanks for making these changes. I use flake8 to lint my Python code. For this file, it will complain about some trailing white-space, and about some module-level imports . I wouldn't worry about either of these too much (though you might want to configure your text editor to eliminate trailing white-space).

Here's a couple you might want to fix, though

doc/examples/fiber_to_bundle_coherence.py:43:1: F401 'op' imported but unused
doc/examples/fiber_to_bundle_coherence.py:44:1: F401 'nib' imported but unused

As for the line you mentioned, I am not getting any flake8 errors on that, and I don't see any problems with that line. I vote to keep it as is.

@stephanmeesters

This comment has been minimized.

Contributor

stephanmeesters commented Jul 20, 2016

Hi @arokem thanks for having a look, I removed the redundant imports.

@coveralls

This comment has been minimized.

coveralls commented Jul 20, 2016

Coverage Status

Coverage remained the same at 82.908% when pulling 1b34aec on stephanmeesters:fbc_measures_update_pr into fbad873 on nipy:master.

@arokem

This comment has been minimized.

Member

arokem commented Jul 20, 2016

LGTM

@arokem arokem merged commit c09a194 into nipy:master Jul 20, 2016

4 checks passed

codecov/patch Coverage not affected when comparing fbad873...1b34aec
Details
codecov/project 80.84% (+0.00%) compared to fbad873
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 82.908%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment