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: Fix GP estimation error analysis script #30

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jhlegarreta
Copy link
Contributor

  • ENH: Fix return value type annotation in GP error analysis script func
  • ENH: Use a boolean to tell whether to serialize the pandas df index
  • BUG: Provide missing position arg to local cross validation function
  • ENH: Add type annotation for variable in GP estimation error analysis

Fix return value type annotation in GP error analysis script function.

Fixes:
```
scripts/dwi_gp_estimation_error_analysis.py:78: error:
 Incompatible return value type (got "ndarray[Any, Any]",
 expected "dict[int, list[tuple[ndarray[Any, Any], ndarray[Any, Any], ndarray[Any, Any], ndarray[Any, Any]]]]")  [return-value]
```

raised for example in:
https://github.com/nipreps/nifreeze/actions/runs/12437972140/job/34728973936#step:8:111
Use a boolean to tell whether to serialize the pandas dataframe index.

Fixes:
```
scripts/dwi_gp_estimation_error_analysis.py:220: error:
 No overload variant of "to_csv" of "NDFrame" matches argument types "Any", "str", "None", "str"  [call-overload]
scripts/dwi_gp_estimation_error_analysis.py:220: note: Possible overload variants:
scripts/dwi_gp_estimation_error_analysis.py:220: note:     def (...)
```

raised for example in:
https://github.com/nipreps/nifreeze/actions/runs/12437972140/job/34728973936#step:8:121
Provide missing position argument to local cross validation function in
GP estimation error analysis script.

Fixes
```
scripts/dwi_gp_estimation_error_analysis.py:210: error:
 Missing positional argument "gpr" in call to "cross_validate"  [call-arg]
scripts/dwi_gp_estimation_error_analysis.py:210: error:
 Unsupported operand types for * ("float" and "dict[int, list[tuple[ndarray[Any, Any], ndarray[Any, Any], ndarray[Any, Any], ndarray[Any, Any]]]]")  [operator]
scripts/dwi_gp_estimation_error_analysis.py:210: error:
 Argument 4 to "cross_validate" has incompatible type "DiffusionGPR"; expected "int"  [arg-type]
scripts/dwi_gp_estimation_error_analysis.py:211: error:
 "float" has no attribute "tolist"  [attr-defined]
scripts/dwi_gp_estimation_error_analysis.py:212: error:
 Argument 1 to "len" has incompatible type "float"; expected "Sized"  [arg-type]
scripts/dwi_gp_estimation_error_analysis.py:213: error:
 Argument 1 to "len" has incompatible type "float"; expected "Sized"  [arg-type]
scripts/dwi_gp_estimation_error_analysis.py:214: error:
 Argument 1 to "len" has incompatible type "float"; expected "Sized"  [arg-type]
scripts/dwi_gp_estimation_error_analysis.py:215: error:
 Argument 1 to "len" has incompatible type "float"; expected "Sized"  [arg-type]
```

raised for example in:
https://github.com/nipreps/nifreeze/actions/runs/12437972140/job/34728973936#step:8:113
@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Dec 22, 2024

Marking as draft:

Add type annotation for local variable `scores`  in GP estimation error
analysis script.

Fixes:
```
scripts/dwi_gp_estimation_error_analysis.py:207:
 error: Need type annotation for "scores"  [var-annotated]
```

raised for example in:
https://github.com/nipreps/nifreeze/actions/runs/12437972140/job/34728973936#step:8:112
Copy link

codecov bot commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.60%. Comparing base (0dadde7) to head (a19c66e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #30   +/-   ##
=======================================
  Coverage   67.60%   67.60%           
=======================================
  Files          20       20           
  Lines         991      991           
  Branches      131      131           
=======================================
  Hits          670      670           
  Misses        267      267           
  Partials       54       54           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jhlegarreta
Copy link
Contributor Author

Most of these changes were addressed in PR #28. The only missing part is that the complaint in #30 (comment) seems legitimate, so ignoring the warning https://github.com/nipreps/nifreeze/pull/28/files#diff-e16a8632d36e294b24752d0ef9e6ea37a63e390a26045e188d8d5d4d0a9dc8ceR83 may not be the best solution. We are likely to revamp this script/the scripts as we make progress, so we will revisit this at due time. Keeping it open as a reminder.

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.

2 participants