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

BUG: Fix Cython one-liner non-trivial type declaration warnings. #1706

Merged

Conversation

@jhlegarreta
Copy link
Contributor

commented Dec 25, 2018

Fix Cython one-liner non-trivial type declaration warnings. Fixes

dipy/tracking/distances.pyx:246:14: Non-trivial type declarators in shared
declaration (e.g. mix of pointers and values). Each pointer declaration
should be on its own line.

and similars.

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2018

Although this is just a matter of styles or tastes, the CI output is read many more times than the Cython code is read/written, so I guess it it is worthwhile removing some clutter from the CI log.

Not sure whether this will change with Cython versions or else, whether it can actually be addressed in some more elegant way.

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2018

The reported test failure is related to #1698.

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:FixCythonNonTrivialTypeDeclarationWarnings branch from 7fcecf9 to 43d35c1 Dec 30, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Dec 30, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@bdb2b97). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1706   +/-   ##
=========================================
  Coverage          ?   84.23%           
=========================================
  Files             ?      114           
  Lines             ?    13556           
  Branches          ?     2139           
=========================================
  Hits              ?    11419           
  Misses            ?     1640           
  Partials          ?      497
BUG: Fix Cython one-liner non-trivial type declaration warnings.
Fix Cython one-liner non-trivial type declaration warnings. Fixes
```
dipy/tracking/distances.pyx:246:14: Non-trivial type declarators in shared
declaration (e.g. mix of pointers and values). Each pointer declaration
should be on its own line.
```

and similars.

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:FixCythonNonTrivialTypeDeclarationWarnings branch from 43d35c1 to 8edc126 Dec 30, 2018

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Dec 31, 2018

Ready to be merged then !

@arokem

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

Sorry for the delay taking a look here. The code itself actually looks more readable like this to me, so I am +1 for this change. Let's see if anyone else has opinions on this. Otherwise, we can merge it at the end of this week.

@skoudoro

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

+1 for merging. This PR reminds me #583. We should create this document and add this rule

@arokem

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

OK. Since we have two 👍 for merging, I will go ahead with this.

Thanks @jhlegarreta!

@arokem arokem merged commit ef0ea81 into nipy:master Jan 2, 2019

4 checks passed

codecov/patch Coverage not affected.
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Jan 2, 2019

Thanks both.

I would not mind to contribute with a Cython style guide.

To be honest, I have not written a single Cython line (yet), so I'd need help from you. When opening the DIPY Cython files, PEP8 points at a few lines. So having a look at those could be useful when writing the guide if we adopt PEP8 as Elef suggested in #583. We could start with an initial version, and thus, a baseline over which make improvements would exist as we come along with related issues/questions.

So we can discuss further in #583 if you wish.

@jhlegarreta jhlegarreta deleted the jhlegarreta:FixCythonNonTrivialTypeDeclarationWarnings branch Jan 2, 2019

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