Pin Cython version for cython-lint pre-commit hook - #23767
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe Cython constraints
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized dependency-pinning change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bdice
left a comment
There was a problem hiding this comment.
This seems like a proper solution until we want to update our Cython pinning across the board.
|
The conda-python-build job got cython 3.3.0 https://github.com/NVIDIA/cudf/actions/runs/32601335885/job/97139678290?pr=23767#step:11:610 │ │ Error compiling Cython file:
│ │ ------------------------------------------------------------
│ │ ...
│ │ The median aggregation.
│ │ """
│ │ return Aggregation.from_libcudf(move(make_median_aggregation[aggregation]()))
│ │
│ │
│ │ cpdef Aggregation quantile(list quantiles: list[float], interpolation interp = interpolation.LINEAR):
│ │ ^
│ │ ------------------------------------------------------------
│ │ $SRC_DIR/python/pylibcudf/pylibcudf/aggregation.pyx:431:43: Signature does not agree with previous declarationI think we're missing a pin here. I'll check for others and will push a commit (done in b55dd25, for all the conda recipes including a host dependency on cython). |
|
Blocking #23738 as well |
|
/merge |
Description
cython-lint job is currently failing CI. Two things going on:
AttributeError: 'DictComprehensionAppendNode' object has no attribute 'value_expr'with Cython3.3.0MarcoGorelli/cython-lint#201If the lack of pinning was an oversight, we can merge this PR regardless of when the cython-lint issue gets fixed and a new release is cut. If not, this should be treated as a temporary workaround. And may not be nescessary at all depending on when the issue is fixed upstream.
Checklist