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

PEP8: Fix pep8 in segment #1025

Merged
merged 5 commits into from Jun 13, 2016

Conversation

Projects
None yet
5 participants
@ghoshbishakh
Member

ghoshbishakh commented Mar 31, 2016

fixes #883

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Apr 1, 2016

This is already being addressed by #953. Can you make a PR against @souravsingh's one?

@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Apr 2, 2016

@manu-tej I saw that it was old and failing tests so made this one. Ill look into that if souravsingh does not respond.

@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Apr 18, 2016

@MarcCote is #953 still active? How can I make a PR against that one as you mentioned?

@souravsingh

This comment has been minimized.

Contributor

souravsingh commented Apr 18, 2016

@ghoshbishakh There are a lot of comments made by @MarcCote which I am working on fixing in the PR

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Apr 18, 2016

@ghoshbishakh: when making a new PR, click on the link "compare across forks". Then select souravsingh/dipy and select his branch fix-segment. That will create a PR that @souravsingh will have to accept. Doing so, it will update his branch with the changes you proposed.
@souravsingh: don't hesitate to ask if you have any question regarding the comments.

@MarcCote MarcCote referenced this pull request Jun 9, 2016

Merged

Additional PEP8 fixes #1

@coveralls

This comment has been minimized.

coveralls commented Jun 10, 2016

Coverage Status

Changes Unknown when pulling 61ad2d4 on ghoshbishakh:segment into * on nipy:master*.

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Jun 10, 2016

@arokem I'll let you have a look at the changes then I'll merge it if everything is fine.

References
----------
E.Garyfallidis, "Towards an accurate brain tractography",

This comment has been minimized.

@arokem

arokem Jun 10, 2016

Member

While we're at it, can we reformat this to a sphinx reference? Also, I believe we can refer to the Quickbundles paper (http://www.ncbi.nlm.nih.gov/pmc/articles/PMC3518823/) here, rather than to Eleftherios's thesis.

This comment has been minimized.

@MarcCote

MarcCote Jun 10, 2016

Contributor

Yes, I remember the discussion we had on the other PR. You are right, let's fix it now.

@arokem

This comment has been minimized.

Member

arokem commented Jun 10, 2016

Looks good. What do you think about that reference? I am aware that this is code that is deprecated and should be removed soon (now?), but that's just a small additional fix.

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Jun 10, 2016

Made a small PR ghoshbishakh#2 for @ghoshbishakh. Yes, removing quickbundles.py is on my TODO :P. I remember @Garyfallidis suggested I wait for some new changes in the workflows but I think now everything is in place to make the change.

Merge pull request #2 from MarcCote/fix_pep8_segment
DOC: fixed QuickBundles reference
@coveralls

This comment has been minimized.

coveralls commented Jun 10, 2016

Coverage Status

Changes Unknown when pulling 7c9d47d on ghoshbishakh:segment into * on nipy:master*.

@MarcCote MarcCote merged commit 7c236b6 into nipy:master Jun 13, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 82.864%
Details

@MarcCote MarcCote referenced this pull request Jun 13, 2016

Closed

FIX: PEP8 for segment #953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment