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

DOC: Fix missing reference to QuickBundles paper. #1437

Merged
merged 1 commit into from Mar 6, 2018

Conversation

Projects
4 participants
@jhlegarreta
Contributor

jhlegarreta commented Feb 19, 2018

Fix missing reference to QuickBbundles paper in the "Enhancing
QuickBundles with different metrics and features" example:
http://nipy.org/dipy/examples_built/segment_extending_clustering_framework.html#example-segment-extending-clustering-framework

Fixes #1371.

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 19, 2018

@skoudoro @Garyfallidis Please review this PR. Thanks.

I did a search for QuickBundles in the doc\examples folder and only the following files contain the string:

segment_clustering_features.py
segment_clustering_metrics.py
segment_extending_clustering_framework.py 
segment_quickbundles.py

So I think the PR closes the referenced issue.

If you happen to know another example that uses QuickBundle behind the scenes, please let me know to incorporate it to this PR.

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 19, 2018

Build failures are

  • Related to #1435
  • TypeError: type numpy.ndarray doesn't define __round__ method in dipy.reconst.tests.test_dti.test_mask

As for the latter, is somebody having a look at it, or should I have a look at it?

@arokem

This comment has been minimized.

Member

arokem commented Feb 19, 2018

Is the latter somehow related to #1399/ #1400 0?

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 20, 2018

@arokem Certainly it looks like it is. However, #1400 was merged, and hence I do not see why the error should persist.

@skoudoro skoudoro added this to Needs a review in Dipy 0.14.0 Mar 1, 2018

@skoudoro

This comment has been minimized.

Member

skoudoro commented Mar 3, 2018

Hi @jhlegarreta, can you rebase all your PR? we just fixed the Cython problem and most of your PR seems ready to be merged. Thanks !

Jon Haitz Legarreta Gorroño
DOC: Fix missing reference to QuickBundles paper.
Fix missing reference to QuickBbundles paper in the "Enhancing
QuickBundles with different metrics and features" example:
http://nipy.org/dipy/examples_built/segment_extending_clustering_framework.html#example-segment-extending-clustering-framework

Fixes #1371.

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:FixQuickBundlesReferencesMissingInExamples branch from 5553163 to 97a7f2e Mar 5, 2018

@codecov-io

This comment has been minimized.

codecov-io commented Mar 5, 2018

Codecov Report

Merging #1437 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1437      +/-   ##
==========================================
- Coverage   87.42%   87.41%   -0.02%     
==========================================
  Files         239      239              
  Lines       30579    30579              
  Branches     3291     3291              
==========================================
- Hits        26735    26731       -4     
- Misses       3075     3078       +3     
- Partials      769      770       +1
Impacted Files Coverage Δ
dipy/reconst/forecast.py 90.15% <0%> (-2.08%) ⬇️

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 6a78d50...97a7f2e. Read the comment docs.

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Mar 5, 2018

@skoudoro 97a7f2e was rebased on master. Thanks for working on the Cython issues !

@skoudoro

This comment has been minimized.

Member

skoudoro commented Mar 6, 2018

LGTM, I do not see any other occurrence too. Thanks @jhlegarreta! merging !

@skoudoro skoudoro merged commit 783dc06 into nipy:master Mar 6, 2018

2 of 3 checks passed

codecov/project 87.41% (-0.02%) compared to 6a78d50
Details
codecov/patch Coverage not affected when comparing 6a78d50...97a7f2e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Dipy 0.14.0 automation moved this from Needs a review to Done Mar 6, 2018

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1437 from jhlegarreta/FixQuickBundlesReferenc…
…esMissingInExamples

DOC: Fix missing reference to QuickBundles paper.

@jhlegarreta jhlegarreta deleted the jhlegarreta:FixQuickBundlesReferencesMissingInExamples branch Oct 10, 2018

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