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

ENH: Add FS transform regression #74

Merged
merged 2 commits into from
Mar 20, 2020
Merged

Conversation

mgxd
Copy link
Member

@mgxd mgxd commented Mar 19, 2020

Tests are passing locally, but a bit iffy on the results.

@pull-assistant
Copy link

pull-assistant bot commented Mar 19, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     WIP: Add FS transform regression

     ENH: hack to add FS license

Powered by Pull Assistant. Last update ebcafde ... ef4e078. Read the comment docs.

@mgxd mgxd requested review from oesteban and effigies March 19, 2020 21:17
Copy link
Collaborator

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Looks good, but there must be some discrepancy with your local installation.

@effigies
Copy link
Member

Looks like we're not passing the freesurfer license into the Docker container?

@codecov-io
Copy link

codecov-io commented Mar 20, 2020

Codecov Report

Merging #74 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   98.63%   98.63%   +<.01%     
==========================================
  Files          11       11              
  Lines         950      951       +1     
  Branches      129      129              
==========================================
+ Hits          937      938       +1     
  Misses          7        7              
  Partials        6        6
Flag Coverage Δ
#unittests 98.63% <ø> (ø) ⬆️
Impacted Files Coverage Δ
nitransforms/io/lta.py 99.32% <0%> (ø) ⬆️
nitransforms/linear.py 96.07% <0%> (+0.02%) ⬆️

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 4acfdbc...ef4e078. Read the comment docs.

@mgxd
Copy link
Member Author

mgxd commented Mar 20, 2020

there must be some discrepancy with your local installation

WDYM?

@oesteban
Copy link
Collaborator

WDYM

The license - you had it only locally :D. Merging...

@oesteban oesteban merged commit 02b5426 into nipy:master Mar 20, 2020
@mgxd mgxd deleted the enh/fs-linear-test branch March 20, 2020 15:17
oesteban added a commit that referenced this pull request Mar 20, 2020
oesteban added a commit to oesteban/nitransforms that referenced this pull request Mar 28, 2020
This PR starts the implementation of "collapsing" series of transforms,
for the linear case.

It is tested on real data added in the context of nipy#66, nipy#74 and nipy#75.
For the moment does not cover unintended use-cases such as collapsing
transform sequences containing one or more nonlinear transforms.

Resolves: nipy#88
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