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

Fix several warnings #364

Merged
merged 11 commits into from
Dec 8, 2023
Merged

Fix several warnings #364

merged 11 commits into from
Dec 8, 2023

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Dec 8, 2023

Closes none, but works on #344.

Changes proposed in this pull request

  • Replace pkg_resources with aslprep.data.load.
  • Add rcond to numpy.linalg.lstsq calls.
  • Properly use hue in seaborn calls.
  • Set keep_masked_labels to True in NiftiLabelsMasker calls.
    • This change will raise an error in Nilearn 0.15, but that will make it easier to detect and fix.

WARNING  py.warnings:_warnings.py:37 WARNING: Passing `palette` without assigning `hue` is deprecated and will be removed in v0.14.0. Assign the `x` variable to `hue` and set `legend=False` for the same effect.
WARNING  py.warnings:_warnings.py:37 WARNING: `rcond` parameter will change to the default of machine precision times ``max(M, N)`` where M and N are the input matrix dimensions.
To use the future default and silence this warning we advise to pass `rcond=None`, to keep using the old, explicitly pass `rcond=-1`.
This change will raise an error in Nilearn 0.15, but that will make it easier to detect and fix.

WARNING  py.warnings:_warnings.py:37 WARNING: Applying "mask_img" before signal extraction may result in empty region signals in the output. These are currently kept. Starting from version 0.13, the default behavior will be changed to remove them by setting "keep_masked_labels=False". "keep_masked_labels" parameter will be removed in version 0.15.
/src/aslprep/aslprep/tests/test_cli.py:7: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
    from pkg_resources import resource_filename as pkgrf
@tsalo tsalo added the bug Something isn't working label Dec 8, 2023
aslprep/cli/workflow.py Outdated Show resolved Hide resolved
tsalo and others added 2 commits December 8, 2023 14:31
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
aslprep/cli/workflow.py Outdated Show resolved Hide resolved
aslprep/cli/workflow.py Outdated Show resolved Hide resolved
aslprep/cli/workflow.py Outdated Show resolved Hide resolved

# NOTE: Modified for aslprep's purposes
aslprep_spec = loads(Path(pkgrf("aslprep", "data/aslprep_bids_config.json")).read_text())
aslprep_spec = loads(load_data.readable("aslprep_bids_config.json").read_text())
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I would pretty much only use readable() in the form load_data.readable(...).read_<text|bytes>(), as otherwise you need to check that whoever you're passing it to is only going to use read_text() or read_bytes().

aslprep/tests/test_cli.py Outdated Show resolved Hide resolved
aslprep/tests/tests.py Outdated Show resolved Hide resolved
aslprep/utils/atlas.py Outdated Show resolved Hide resolved
docs/workflows.rst Outdated Show resolved Hide resolved
docs/workflows.rst Outdated Show resolved Hide resolved
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@effigies
Copy link
Contributor

effigies commented Dec 8, 2023

Thanks so much @effigies! I had no clue data.load could do so much.

https://www.nipreps.org/niworkflows/master/api/niworkflows.data.html

If you add aslprep.data to your API docs, you'll get something similar, listing the available imports for you as well.

@codecov-commenter
Copy link

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (67607e1) 78.81% compared to head (f1431c0) 79.09%.
Report is 2 commits behind head on main.

Files Patch % Lines
aslprep/utils/atlas.py 0.00% 4 Missing ⚠️
aslprep/cli/workflow.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
+ Coverage   78.81%   79.09%   +0.28%     
==========================================
  Files          39       39              
  Lines        4149     4148       -1     
  Branches      600      600              
==========================================
+ Hits         3270     3281      +11     
+ Misses        685      673      -12     
  Partials      194      194              

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

@tsalo tsalo merged commit eec466b into PennLINC:main Dec 8, 2023
25 checks passed
@tsalo tsalo deleted the fix-warnings branch December 8, 2023 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants