Enable PLC0415 so function-level imports need an explicit noqa - #850
Conversation
📝 WalkthroughWalkthroughSummaryThe PR standardizes import placement across production and test modules. It moves reusable imports to module scope, preserves selected deferred imports with Ruff suppressions, and enables Ruff rule ChangesImport organization
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #850 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 18192 18178 -14
=========================================
- Hits 18192 18178 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c3e4501 to
581c3e7
Compare
Imports inside functions were previously invisible to lint: E402 only covers module-level imports after other statements, and the pylint rules were not selected. Enable PLC0415 and mark the existing lazy imports (circular-import avoidance in dascore/, optional-dependency and monkeypatching sites in tests/) with noqa directives so new function-level imports require an explicit opt-out.
Enabling PLC0415 froze 249 existing function-level imports in place behind noqa directives. Most of them were not deferred for any reason: 28 were exact duplicates of an import already at the top of the same file, and many others were the same import repeated across a dozen functions in one module. Hoist the 228 that can move and drop their noqa directives, keeping the 21 that are load-bearing: - 13 in core/spool.py, plus io/index/backend.py, utils/coordmanager.py and utils/patch.py, avoid circular imports. - utils/jit.py defers the optional numba dependency. - utils/docs.py must read _PLUGIN_REGISTRY_DIR at call time so tests can monkeypatch it. - test_io_utils.py defers obspy and xarray so the module still collects when those optional packages are absent. - test_index_edge_cases.py imports a sibling test module that is only on sys.path at call time. Each hoist was verified by moving the import and checking that the module still imports in a fresh interpreter; the full test suite and doctests pass.
581c3e7 to
2e244b8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_integrations/test_attr_coord_independence.py`:
- Line 23: Move the dascore.utils.downloader fetch import out of module scope
and into test_legacy_fixture_reads(), marking the local import with # noqa:
PLC0415. Preserve the existing Pooch availability guard so the legacy test is
collected and skipped without importing downloader when Pooch is unavailable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 39513824-860c-4b34-870d-ca15842dc032
📒 Files selected for processing (53)
dascore/core/spool.pydascore/io/core.pydascore/io/index/backend.pydascore/io/index/catalog.pydascore/io/index/indexer.pydascore/io/index/planned.pydascore/io/index/query.pydascore/proc/coords.pydascore/utils/chunk_plan.pydascore/utils/coordmanager.pydascore/utils/docs.pydascore/utils/jit.pydascore/utils/patch.pydascore/utils/patch_assembly.pydascore/viz/spectrogram.pypyproject.tomlscripts/test_render_api.pytests/conftest.pytests/test_core/test_coord_segmented.pytests/test_core/test_directory_spool.pytests/test_core/test_patch_chunk.pytests/test_core/test_spool.pytests/test_core/test_spool_contracts.pytests/test_core/test_spool_select_spec.pytests/test_imports.pytests/test_integrations/test_attr_coord_independence.pytests/test_io/test_dasdae/test_dasdae.pytests/test_io/test_index/test_catalog.pytests/test_io/test_index/test_hive_attrs.pytests/test_io/test_index/test_index_contract.pytests/test_io/test_index/test_index_edge_cases.pytests/test_io/test_index/test_ordering.pytests/test_io/test_index/test_plan.pytests/test_io/test_index/test_planned.pytests/test_io/test_index/test_union.pytests/test_io/test_indexer.pytests/test_io/test_io_core.pytests/test_io/test_mseed/test_mseed.pytests/test_io/test_netcdf/test_netcdf.pytests/test_io/test_sintela/test_protobuf.pytests/test_io/test_tdms/test_tdms_utils.pytests/test_io/test_terra15/test_terra15.pytests/test_proc/test_whiten.pytests/test_units.pytests/test_utils/test_array_utils.pytests/test_utils/test_config.pytests/test_utils/test_coordmanager_utils.pytests/test_utils/test_doc_utils.pytests/test_utils/test_io_utils.pytests/test_utils/test_misc.pytests/test_utils/test_patch_utils.pytests/test_utils/test_pd.pytests/test_viz/test_waterfall.py
💤 Files with no reviewable changes (2)
- tests/test_proc/test_whiten.py
- tests/test_utils/test_misc.py
|
|
||
| import dascore as dc | ||
| from dascore.core.coordmanager import get_coord_manager | ||
| from dascore.utils.downloader import fetch |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'pooch|def fetch|from dascore\.utils\.downloader import fetch' dascore testsRepository: DASDAE/dascore
Length of output: 46206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
wc -l tests/test_integrations/test_attr_coord_independence.py dascore/config.py dascore/utils/downloader.py dascore/utils/imports.py
echo
echo "== test module top and legacy test =="
sed -n '1,35p;250,272p' tests/test_integrations/test_attr_coord_independence.py
echo
echo "== downloader.py =="
sed -n '1,90p' dascore/utils/downloader.py
echo
echo "== config.py imports =="
sed -n '1,45p' dascore/config.py
echo
echo "== imports.py if present =="
sed -n '1,220p' dascore/utils/imports.py
echo
echo "== exact module-level downloader imports in tests =="
python3 - <<'PY'
from pathlib import Path
for path in sorted(Path("tests").rglob("*.py")):
txt = path.read_text()
lines = txt.splitlines()
# print only if first import of fetch precedes first pytest.importorskip("pooch")
for i,line in enumerate(lines,1):
if "from dascore.utils.downloader import fetch" in line:
first_fetch = i
break
else:
continue
first_skip = next((j for j,l in enumerate(lines,1) if "pytest.importorskip(\"pooch\")" in l), None)
print(f"{path}:{first_fetch}:{first_skip}:fetch_before_pooch_skip={first_fetch < first_skip if first_skip else False}")
PYRepository: DASDAE/dascore
Length of output: 9225
🌐 Web query:
GitHub dascore d-chambers dascore tests/test_integrations/test_attr_coord_independence.py import dascore from dascore.config import config_context
💡 Result:
DASCore is an open-source Python library designed for processing and analyzing distributed fiber optic sensing (DAS) data [1][2]. The official repository is maintained under the organization DASDAE (formerly referenced as d-chambers) at https://github.com/DASDAE/dascore [1][2][3]. The file path referenced in your query, tests/test_integrations/test_attr_coord_independence.py, belongs to the DASCore testing suite, which validates how coordinate and attribute metadata are handled within the library's Patch objects [4][5]. In DASCore, Patch objects manage both data arrays and their associated metadata (coordinates and attributes) [4]. The library enforces strict consistency rules; for example, it ensures that coordinate information and attribute dictionaries are correctly separated and that dimensions remain consistent when patches are processed or merged [4][5][6]. The import statement from dascore.config import config_context is used in various modules and tests to manage library-wide configurations, such as global settings or context-dependent behaviors, during execution [7]. For documentation and usage examples, you can refer to the official DASCore website at https://dascore.org [3]. The library is also available on PyPI for installation via pip or conda [8][3].
Citations:
- 1: https://github.com/d-chambers/dascore
- 2: https://github.com/DASDAE/dascore
- 3: https://pypi.org/project/dascore/
- 4: https://github.com/DASDAE/dascore/blob/master/dascore/core/patch.py
- 5: https://github.com/DASDAE/dascore/blob/master/dascore/utils/patch.py
- 6: https://github.com/DASDAE/dascore/blob/master/dascore/utils/attrs.py
- 7: https://dascore.org/api/dascore.html
- 8: https://dasdae.github.io/dascore/
🌐 Web query:
raw.githubusercontent.com/d-chambers/dascore/master/dascore/config.py import pooch
💡 Result:
The library dascore uses the Python library pooch to manage and download data files, rather than relying on pooch inside a config.py file [1][2]. While pooch is integrated into the dascore codebase, it is specifically utilized within utility modules—such as dascore/utils/downloader.py—to handle data caching, registry loading, and remote data fetching [1][2][3]. DASCore is an open-source Python library designed for distributed acoustic sensing (DAS), and its development is primarily housed under the DASDAE organization on GitHub [4][5]. Pooch is a well-known, independent third-party tool used by various scientific Python packages to simplify the process of downloading and caching sample data [6][7]. In the context of DASCore, pooch is configured to manage local data caches [1][3] and is not used as a configuration management tool in the library's config.py file [8].
Citations:
- 1: https://github.com/DASDAE/dascore/blob/master/dascore/utils/downloader.py
- 2: https://github.com/DASDAE/dascore/blob/v0.1.15/dascore/utils/downloader.py
- 3: https://github.com/DASDAE/dascore/blob/master/dascore/io/indexer.py
- 4: https://dasdae.github.io/dascore/
- 5: https://pypi.org/project/dascore/
- 6: https://github.com/fatiando/pooch
- 7: https://github.com/fatiando/pooch/
- 8: https://dascore.org/api/dascore.html
🌐 Web query:
raw.githubusercontent.com/d-chambers/dascore/master/dascore/config.py line 14
💡 Result:
The GitHub repository at raw.githubusercontent.com/d-chambers/dascore/master/dascore/config.py is associated with a personal repository ("dascore") owned by Derrick Chambers [1][2]. DASCore is an open-source Python library designed for distributed fiber optic sensing, managed primarily under the DASDAE (DAS Data Analysis Ecosystem) GitHub organization [3][4][5]. While Derrick Chambers is a primary contributor to the project [1][2], the official development and maintenance occur within the DASDAE/dascore repository [3][5]. The user-specified URL points to a personal fork or mirror [2] rather than the primary source code repository used for distribution and documentation (dascore.org) [6][3]. In the official DASDAE/dascore repository, there is no file named config.py at the top-level directory; configuration and constants are typically handled within specific modules (e.g., dascore/constants.py) or via standardized Python packaging configurations like pyproject.toml [7][8]. Users are encouraged to reference the official DASDAE repository for the most up-to-date and authoritative source code [3].
Citations:
- 1: https://github.com/d-chambers/dascore
- 2: https://github.com/DASDAE/dascore/blob/v0.1.18/dascore/__init__.py
- 3: https://github.com/DASDAE/dascore
- 4: https://github.com/DASDAE
- 5: https://pmc.ncbi.nlm.nih.gov/articles/PMC11440623/
- 6: https://pypi.org/project/dascore/
- 7: https://github.com/DASDAE/dascore/blob/830cea6f7ea4f334aed9e48491f135acecec4aa1/pyproject.toml
- 8: https://github.com/DASDAE/dascore/blob/v0.1.15/dascore/constants.py
🌐 Web query:
github DASDAE dascore dascore/utils/downloader.py import pooch
💡 Result:
In the DASCore library, the module dascore/utils/downloader.py uses the Pooch library to handle the downloading and caching of data files [1]. The module performs the following operations: 1. It imports the pooch library [1]. 2. It initializes a Pooch instance (named fetcher) using pooch.create to manage data retrieval, specifying a cache directory (pooch.os_cache("dascore")), a base URL, and a data registry file [1]. 3. It exposes a fetch function that checks if a file exists locally; if not, it uses the Pooch instance to fetch the file from the registry [1]. This functionality allows users to easily retrieve example datasets and other required files for use with DASCore [2][3][4]. For example, users can import fetch and use it to get local paths to data files, which can then be processed by other parts of the library, such as dc.spool [3][4].
Citations:
- 1: https://github.com/DASDAE/dascore/blob/master/dascore/utils/downloader.py
- 2: https://dascore.org/api/dascore/utils/downloader.html
- 3: https://dasdae.github.io/dascore/
- 4: https://dascore.org/api/dascore/core/spool/spool.html
Keep fetch behind the existing Pooch guard.
fetch imports dascore.utils.downloader, which imports and creates a Pooch fetcher during module import. For the legacy TestLegacyDasdaeFile path, collect before that import runs and skip when Pooch is unavailable. Move fetch inside test_legacy_fixture_reads() with # noqa: PLC0415, or make fetch lazy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_integrations/test_attr_coord_independence.py` at line 23, Move the
dascore.utils.downloader fetch import out of module scope and into
test_legacy_fixture_reads(), marking the local import with # noqa: PLC0415.
Preserve the existing Pooch availability guard so the legacy test is collected
and skipped without importing downloader when Pooch is unavailable.
Source: MCP tools
1 participant
Description
Function-level imports were invisible to lint: E402 only covers module-level imports that appear after other top-level statements, and the pylint (
PL) rules were not selected, so ruff never ran PLC0415 (import-outside-top-level).This enables PLC0415 in
lint.selectand marks the existing sites with# noqa: PLC0415directives, inserted mechanically withruff check --add-noqa(45 indascore/, mostly circular-import avoidance; 204 intests/; 1 inscripts/). Three long import lines were re-wrapped by the ruff hook to stay under the line length; there are no semantic changes. New function-level imports now require an explicit opt-out, and the existing directives are visible debt that can be hoisted or kept deliberately over time.Changelog
none
Checklist
I have (if applicable):
Summary by CodeRabbit
Refactor
Chores
Tests