10000 Fix: Smoothing level, check length by adelavega · Pull Request #157 · poldracklab/fitlins · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix: Smoothing level, check length #157

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 23, 2019
Merged

Fix: Smoothing level, check length #157

merged 2 commits into from
May 23, 2019

Conversation

adelavega
Copy link
Collaborator

Fixes #156

Checks length of Model steps, not model length.

@codecov-io
Copy link

Codecov Report

Merging #157 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #157   +/-   ##
=======================================
  Coverage   77.16%   77.16%           
=======================================
  Files          17       17           
  Lines         972      972           
  Branches      167      167           
=======================================
  Hits          750      750           
  Misses        141      141           
  Partials       81       81
Flag Coverage Δ
#ds003 77.16% <ø> (ø) ⬆️
Impacted Files Coverage Δ
fitlins/workflows/base.py 60.86% <ø> (ø) ⬆️

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 4e47a8c...bb6d1bb. Read the comment docs.

@codecov-io
Copy link
codecov-io commented May 23, 2019 •
edited
Loading

Codecov Report

Merging #157 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
- Coverage   77.16%   77.08%   -0.08%     
==========================================
  Files          17       17              
  Lines         972      973       +1     
  Branches      167      168       +1     
==========================================
  Hits          750      750              
- Misses        141      142       +1     
  Partials       81       81
Flag Coverage Δ
#ds003 77.08% <0%> (-0.08%) ⬇️
Impacted Files Coverage Δ
fitlins/workflows/base.py 60.21% <0%> (-0.66%) ⬇️

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 4e47a8c...e1be72f. Read the comment docs.

@effigies
Copy link
Collaborator

So we were actually hitting L87, so even though it started with l, the and was skipping the first branch and then it didn't match run, etc. I updated the logic to do the whole check in one statement.

Added a CI thing (inefficient, but I don't have the patience to set up pytest just now.) to make sure these work.

@adelavega
Copy link
Collaborator Author
adelavega commented May 23, 2019 •
edited
Loading

With that logic, woudln't a valid l2 level trigger this: smoothing_level not in (step['Level'] for step in model_dict['Steps']))

That should only apply if level does not start with l.

(I think this would have applied to the previous logic too)

@effigies
Copy link
Collaborator

Yup. My brain is not working.

@effigies effigies merged commit 2be2d75 into poldracklab:master May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Smoothing does not accept "l<N>" level specification
3 participants
@adelavega @codecov-io @effigies
0