Skip to content

fix more verify-hardcoded-version findings - #7891

Merged
jameslamb merged 1 commit into
NVIDIA:mainfrom
jameslamb:more-precommit
Mar 13, 2026
Merged

fix more verify-hardcoded-version findings#7891
jameslamb merged 1 commit into
NVIDIA:mainfrom
jameslamb:more-precommit

Conversation

@jameslamb

Copy link
Copy Markdown
Member

Similar to #7882

Fixes new verify-hardcoded-version findings on main

In file python/cuml/cuml/internals/validation.py💯66:
             "the feature names of dataframe-like inputs. In cuml 26.06 this "
warning: do not hard-code version, read from VERSION file instead

In file python/cuml/cuml/internals/validation.py💯66:
             "the feature names of dataframe-like inputs. In cuml 26.06 this "

In file python/cuml/tests/ts_datasets/hourly_earnings_by_industry.csv:102:14:
 2014Q1,29.29,26.6,35.07,26.02,29.01,19,17.16,26.57,38.91,40.75,28.84,31.03,34.1
,29.29
warning: do not hard-code version, read from VERSION file instead

In file python/cuml/tests/ts_datasets/hourly_earnings_by_industry.csv:102:14:
 2014Q1,29.29,26.6,35.07,26.02,29.01,19,17.16,26.57,38.91,40.75,28.84,31.03,34.1
,29.29

@jameslamb
jameslamb requested a review from csadorf March 13, 2026 18:49
@jameslamb
jameslamb requested review from a team as code owners March 13, 2026 18:49
@jameslamb jameslamb added the improvement Improvement / enhancement to an existing function label Mar 13, 2026
@jameslamb jameslamb added the non-breaking Non-breaking change label Mar 13, 2026
@jameslamb
jameslamb requested a review from divyegala March 13, 2026 18:49
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Mar 13, 2026
warnings.warn(
"cuml is adding support for `feature_names_in_` for validating "
"the feature names of dataframe-like inputs. In cuml 26.06 this "
"the feature names of dataframe-like inputs. In version 26.06 of cuML this "

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This rewording hits one of the heuristics the linter uses to detect false positives: rapidsai/pre-commit-hooks#112

@coderabbitai

coderabbitai Bot commented Mar 13, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b7c8fff2-dfd3-4905-ae9d-30c9256bf2c6

📥 Commits

Reviewing files that changed from the base of the PR and between c5cd402 and 8c9f168.

📒 Files selected for processing (2)
  • .pre-commit-config.yaml
  • python/cuml/cuml/internals/validation.py

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Chores
    • Updated pre-commit configuration to support CSV and JSON file formats.
    • Refined warning message text for improved clarity and consistency.

Walkthrough

Two minor updates: the .pre-commit-config.yaml file's verify-hardcoded-version rule now includes .csv and .json file extensions in its allowed list, and a warning message in the validation module was reworded for clarity.

Changes

Cohort / File(s) Summary
Pre-commit Configuration
.pre-commit-config.yaml
Extended allowed file extensions in verify-hardcoded-version rule to include .csv and .json alongside existing extensions.
Warning Message Update
python/cuml/cuml/internals/validation.py
Updated warning message text from "In cuml 26.06 this" to "In version 26.06 of cuML this" for improved clarity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • KyleFromNVIDIA
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix more verify-hardcoded-version findings' directly and clearly summarizes the main change - fixing additional hard-coded version findings detected by a pre-commit check.
Description check ✅ Passed The description is directly related to the changeset, explaining the context of the PR (similar to #7882), listing specific affected files, and describing the nature of the warnings being fixed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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

@jcrist

jcrist commented Mar 13, 2026

Copy link
Copy Markdown
Member

These aren't failing on the 26.04 branch but are failing on main - is the pre-commit check different between these branches?

@KyleFromNVIDIA

Copy link
Copy Markdown
Member

The check looks at the VERSION file and works off of that. If VERSION says 26.04 then it looks for 26.04, if it says 26.06 then it looks for 26.06.

@jcrist

jcrist commented Mar 13, 2026

Copy link
Copy Markdown
Member

Since our version strings are a fixed format, could we update the check to look for more than just the current version? Our deprecation warnings usually include the current release version as well as the upcoming one. This would help avoid merging in code that won't cause pre-commit failures until the version is bumped. Just a suggestion.

@KyleFromNVIDIA

Copy link
Copy Markdown
Member

That's not a bad idea. I can have it look at the previous and next minor versions for cases like ucxx, and the previous and next calver versions for the rest of RAPIDS. I'll add it to the list.

@KyleFromNVIDIA

Copy link
Copy Markdown
Member

See rapidsai/pre-commit-hooks#122.

@jameslamb

Copy link
Copy Markdown
Member Author

Admin-merging this, the failures are unrelated (I think they're caused by not having new nightlies for all packages yet).

@jameslamb
jameslamb merged commit 20fbdff into NVIDIA:main Mar 13, 2026
69 of 92 checks passed
@jameslamb
jameslamb deleted the more-precommit branch March 13, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants