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

MAINT: Better config on setup.py (binary operator starting line) #60

Merged
merged 2 commits into from
Feb 20, 2020

Conversation

oesteban
Copy link
Collaborator

No description provided.

@oesteban oesteban requested a review from mgxd February 19, 2020 16:56
@pull-assistant
Copy link

pull-assistant bot commented Feb 19, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     MAINT: Better config on setup.py (binary operator starting line)

     Update nitransforms/tests/test_nonlinear.py

Powered by Pull Assistant. Last update abcb62b ... d39340f. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Feb 19, 2020

Codecov Report

Merging #60 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #60   +/-   ##
=======================================
  Coverage   98.84%   98.84%           
=======================================
  Files          11       11           
  Lines         868      868           
  Branches      111      111           
=======================================
  Hits          858      858           
  Misses          6        6           
  Partials        4        4
Flag Coverage Δ
#unittests 98.84% <100%> (ø) ⬆️
Impacted Files Coverage Δ
nitransforms/io/itk.py 100% <ø> (ø) ⬆️
nitransforms/io/afni.py 98.7% <ø> (ø) ⬆️
nitransforms/base.py 99.4% <100%> (ø) ⬆️

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 81e7a67...d39340f. Read the comment docs.

Copy link
Member

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few comments

@@ -33,5 +33,22 @@ console_scripts =
nt = nitransforms.cli:main

[flake8]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we plan on porting this over to nibabel, let's revert the style checks to match theirs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh right. We might want to push for more strict flake8 checks. @effigies ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go-ahead - good syntax should not be a problem when integrating into nibabel.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry. I saw this on my notifications and then it disappeared. Yeah, we can improve things in nibabel, but being more strict here can't cause problems.

@oesteban oesteban merged commit f8b725f into nipy:master Feb 20, 2020
@oesteban oesteban deleted the sty/W504 branch February 20, 2020 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants