Skip to content

MNT: Address warnings produced by pybids and dependencies#1136

Merged
effigies merged 12 commits intobids-standard:masterfrom
effigies:maint/warnings
May 29, 2025
Merged

MNT: Address warnings produced by pybids and dependencies#1136
effigies merged 12 commits intobids-standard:masterfrom
effigies:maint/warnings

Conversation

@effigies
Copy link
Copy Markdown
Collaborator

Given that we're now testing on nightly builds of scientific Python dependencies, seems like a good time to address warnings.

This addresses warnings both in dependencies and generated by pybids itself. We have behavior that was supposedly deprecated in versions 0.7 and 0.14, but was still around. I've mostly kept helpful messages, but turned them into TypeErrors, which is what they'll be when fully removed.

@codecov
Copy link
Copy Markdown

codecov bot commented May 24, 2025

Codecov Report

Attention: Patch coverage is 73.72881% with 31 lines in your changes missing coverage. Please review.

Project coverage is 89.41%. Comparing base (2891f8d) to head (2a050d8).
Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
src/bids/modeling/statsmodels.py 33.33% 11 Missing and 1 partial ⚠️
src/bids/layout/layout.py 50.00% 8 Missing and 3 partials ⚠️
src/bids/layout/models.py 33.33% 2 Missing and 2 partials ⚠️
src/bids/variables/collections.py 86.95% 2 Missing and 1 partial ⚠️
src/bids/reports/parsing.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1136      +/-   ##
==========================================
- Coverage   89.77%   89.41%   -0.36%     
==========================================
  Files          65       65              
  Lines        7205     7237      +32     
  Branches     1137     1142       +5     
==========================================
+ Hits         6468     6471       +3     
- Misses        536      557      +21     
- Partials      201      209       +8     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@effigies effigies force-pushed the maint/warnings branch 2 times, most recently from 622ce7e to 3a5cd05 Compare May 24, 2025 23:47
Copy link
Copy Markdown
Collaborator Author

@effigies effigies left a comment

Choose a reason for hiding this comment

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

A few notes. @adelavega Do you have some time to review?

The only remaining warnings are ResourceWarnings from unclosed database connections. I tried a few things to reduce those, but couldn't eliminate them, so I'm not addressing them in this PR.

"""

def __init__(self, root=None, validate=True, absolute_paths=True,
def __init__(self, root=None, validate=True, absolute_paths=RemovedOption,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Significant change: Removing absolute_paths from BIDSLayout.__init__ and BIDSLayout.get, deprecated in 0.14.

Comment on lines +126 to +132
if indexer_kwargs:
args = ', '.join(f'{key}={val}' for key, val in indexer_kwargs.items())
msg = (
"Passing kwargs to the BIDSLayoutIndexer is no longer supported. "
f"Pass `indexer=BIDSLayoutIndexer({args})` instead."
)
raise TypeError(msg)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Significant change: No longer passing BIDSLayout kwargs to BIDSLayoutIndexer, deprecated in 0.14.


root = Column(String, primary_key=True)
absolute_paths = Column(Boolean)
absolute_paths = Column(Boolean, default=True) # Removed, but may be in older DBs
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Seemed best to keep this column and just enforce its value.

Comment on lines -223 to -236
def __getattr__(self, attr):
# Ensures backwards compatibility with old File_ namedtuple, which is
# deprecated as of 0.7.
# _ check first to not mask away access to __setstate__ etc.
# AFAIK None of the entities are allowed to start with _ anyways
# so the check is more generic than __
if not attr.startswith('_') and attr in self.entities:
warnings.warn("Accessing entities as attributes is deprecated as "
"of 0.7. Please use the .entities dictionary instead"
" (i.e., .entities['%s'] instead of .%s."
% (attr, attr))
return self.entities[attr]
raise AttributeError("%s object has no attribute named %r" %
(self.__class__.__name__, attr))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Could have promoted the warning to an error, but it's warning been around so long it doesn't seem worth it.

except ImportError:
pytest.skip("Needs bsmschema, available for Python 3.8+")
BIDSStatsModel.parse_obj(model)
BIDSStatsModel.model_validate(model)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pydantic 2 update.

@adelavega
Copy link
Copy Markdown
Collaborator

These changes look reasonable to me! I think we've given ample warning (pun intended)

@effigies effigies merged commit 53f0681 into bids-standard:master May 29, 2025
28 checks passed
effigies added a commit that referenced this pull request Sep 24, 2025
Version 0.20.0 (September 24, 2025)

New feature release in the 0.20.x series.

This release finalizes some deprecations that were started in the 0.7 and 0.14 series.
In particular, passing indexer arguments to ``BIDSLayout`` and disabling
``absolute_paths`` mode in ``BIDSLayout()`` or ``BIDSLayout.get()`` will now error.
To adjust indexing behavior, create an explicit ``BIDSLayoutIndexer()`` and pass
``BIDSLayout(..., indexer=indexer)``.
To access relative paths, use the ``BIDSFile.relative_path`` property on
``BIDSLayout.get()`` return values.

* FIX: Check for attribute that can be absent before assigning (#1165)
* FIX: Accommodate deprecations in the upcoming Pandas 3 release (#1173)
* FIX: Repaired convolution issue that arises when onset values are repeated (#1133)
* ENH: StatsModels: Add `invalid_inputs` parameter to allow dropping of missing data columns to run nodes (#1145)
* ENH: Add missing root-level path patterns (#1169)
* ENH: Parse ``seg-`` entities from dseg or probseg files (#1172)
* DOC: Update link to derivatives config file (#1132)
* MNT: Address warnings produced by pybids and dependencies (#1136)
* MNT: Lock test environment, update with dependabot (#1134)
@effigies effigies deleted the maint/warnings branch October 30, 2025 00:45
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