Skip to content

Add load_with_anchors() helper - #161

Open
KyleFromNVIDIA wants to merge 1 commit into
rapidsai:mainfrom
KyleFromNVIDIA:load_with_anchors
Open

KyleFromNVIDIA wants to merge 1 commit into
rapidsai:mainfrom
KyleFromNVIDIA:load_with_anchors

Conversation

@KyleFromNVIDIA

Copy link
Copy Markdown
Member

Clean up more tech debt by adding a helper deduplicates all of the usage and disposal of AnchorPreservingLoader.

Clean up more tech debt by adding a helper deduplicates all of the
usage and disposal of `AnchorPreservingLoader`.
@KyleFromNVIDIA
KyleFromNVIDIA requested a review from a team as a code owner October 6, 2026 20:22
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: rapidsai/pre-commit-hooks/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 21490752-fb2d-41f9-8154-be38ec5ea9a8
📥 Commits

Reviewing files that changed from the base of the PR and between b6569b7 and a6ac2a5.

📒 Files selected for processing (9)
  • src/rapids_pre_commit_hooks/utils/dependencies_yaml.py
  • src/rapids_pre_commit_hooks/utils/yaml.py
  • tests/rapids_pre_commit_hooks/dependencies/test_cuda_suffixed.py
  • tests/rapids_pre_commit_hooks/dependencies/test_naming_conventions.py
  • tests/rapids_pre_commit_hooks/dependencies/test_use_cuda_wheels.py
  • tests/rapids_pre_commit_hooks/test_alpha_spec.py
  • tests/rapids_pre_commit_hooks/utils/test_dependencies_yaml.py
  • tests/rapids_pre_commit_hooks/utils/test_yaml.py
  • tests/test_testing_utils.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Refactor
    • Standardized YAML parsing across dependency checks and related tests, while preserving anchor information and existing validation behavior.

Walkthrough

The change adds load_with_anchors to parse YAML and return its root node with anchor metadata. Dependency traversal and tests now use this helper instead of managing YAML loaders directly.

Changes

YAML anchor loading

Layer / File(s) Summary
Shared YAML loader
src/rapids_pre_commit_hooks/utils/yaml.py, tests/rapids_pre_commit_hooks/utils/test_yaml.py
load_with_anchors returns a YAML root node and a copy of the first document’s anchors. It disposes of the loader in a finally block. The test checks the returned mapping.
Dependency traversal and test adoption
src/rapids_pre_commit_hooks/utils/dependencies_yaml.py, tests/rapids_pre_commit_hooks/dependencies/*, tests/rapids_pre_commit_hooks/test_alpha_spec.py, tests/rapids_pre_commit_hooks/utils/test_dependencies_yaml.py, tests/test_testing_utils.py
Dependency traversal and tests use load_with_anchors to obtain YAML nodes and, where needed, anchor mappings. They no longer manage loaders directly.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: bdice

Merge Risk: ⚪ Minimal · up to a6ac2

No concrete behavior regression is established in the loader or its migrated callers, so this change presents no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding the load_with_anchors() helper.
Description check ✅ Passed The description explains that the helper consolidates AnchorPreservingLoader usage and disposal, which matches the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

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.

1 participant