Fix datetime day filtering by driavysinus · Pull Request #767 · VectorInstitute/cyclops

driavysinus · 2026-07-08T21:25:42Z

Problem

filter_datetime(..., day=...) currently reads the year component when applying the day filter, so day-of-month slicing can incorrectly return no matches.

Fix

Use pc.day(...) for the day branch and add a focused unit test for day-of-month filtering.

Validation

Z:\uv\uv.exe run --with datasets==2.21.0 --with pytest --with pandas --with pyarrow --with numpy --with psutil python -m pytest tests\cyclops\data\test_slicer_datetime.py -q
python -m py_compile cyclops\data\slicer.py tests\cyclops\data\test_slicer_datetime.py

Note: I used datasets==2.21.0 locally to match the repository's Poetry constraint range and avoid newer datasets API drift.

for more information, see https://pre-commit.ci

driavysinus · 2026-08-03T17:01:05Z

Update on the failing pre-commit.ci check: the datetime change itself is not causing the failure. In the linked run, ruff and ruff-format pass; nbqa-black reformats 11 notebooks unrelated to this PR, and nbqa-ruff applies one additional unrelated fix.

The branch has already gone through an autofix → restore → autofix loop on docs/source/tutorials/nihcxr/monitor_api.ipynb. I have not added the 11 repo-wide notebook changes in order to keep this bug fix scoped.

Could a maintainer advise whether to treat this check as a repository baseline issue, or whether you would prefer a separate repo-wide notebook-formatting PR first?

amrit110 · 2026-08-04T02:50:29Z

Hi @driavysinus, thanks for the fix and for digging into the CI failure! 🙏

You're right — this is a repository baseline issue, not something your change introduced. I just ran pre-commit run nbqa-black --all-files on a clean main locally and it reformats the exact same 11 notebooks, so the nbqa-black/nbqa-ruff failures would show up on any PR right now.

Keeping this PR scoped to the bug fix is the right call — please don't add the repo-wide notebook changes here. We'll take care of the notebook formatting in a separate PR on our end.

Thanks again for the contribution, the pc.day fix and the added test look good! 🎉

…able dependencies (#768) * Remove redundant nbqa hooks that conflict with ruff-format The nbqa-black and nbqa-ruff hooks duplicate what the ruff and ruff-format hooks already do for notebooks (types_or includes jupyter), and black disagrees with ruff-format about blank lines in notebook cells. This caused pre-commit.ci to loop endlessly, reformatting 11 notebooks on every run and blocking unrelated PRs (e.g. #767). With the nbqa hooks removed, the full suite passes on a clean checkout with no notebook changes required. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Regenerate poetry.lock to remove bogus aarch64 platform markers The lock file had been regenerated with Poetry 2.1.1, which narrowed markers to the locking platform and stamped nearly every package with 'platform_system == "Linux" and platform_machine == "aarch64"'. On CI's x86_64 runners poetry therefore installed only ~14 packages, so pre-commit and sphinx-build were missing and the code-check and docs jobs have failed on every run since. Regenerated with Poetry 2.4.1 (same package versions, no updates). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Remove integration tests workflow It targets self-hosted runners (gpu, db) that are no longer available, so the job queues forever and blocks PR checks from completing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Migrate to Python 3.11 and uv, fix version-drift breakages Packaging: - Convert pyproject.toml to PEP 621 with uv dependency groups and the hatchling build backend; replace poetry.lock with uv.lock - Require Python 3.11 (single-version window, >=3.11,<3.12) - Bump torch to ^2.0 (resolves 2.13.0) since torch 1.x has no cp311 wheels; torchvision follows - Security-driven updates now resolvable: urllib3 2.7.0, requests 2.34.2, setuptools 80.x (capped <81 for pybtex's pkg_resources), pytest 9, jupyterlab 4.6 - Pin scikit-learn <1.8 (cyclops imports the private _binary_clf_curve, removed in 1.8) and torchmetrics <1.3 (metric tests are written against its 1.2 zero-division reference behavior) CI and tooling: - Rewrite all workflows to use astral-sh/setup-uv and uv sync; drop poetry everywhere (Dockerfile, README, workflows) - Scope pip-audit ignores to vulnerabilities that are unfixable while alibi/alibi-detect cap pillow<11 and transformers<5, and cycquery caps pyarrow<18 Code fixes for new versions: - sklearn 1.7: precision_recall_curve keyword probas_pred -> y_score - torch 2.9+: index with a tuple of slices in torch_distributed - mypy 1.x updates: remove stale type-ignores, untyped-decorator code, import PIL.Image explicitly, restructure MetricCollection.add_metrics - tests: make OMOP querier construction lazy so importing test_slicer no longer requires a database; use a local RNG in _inject_ignore_index so results don't depend on execution order Full pre-commit suite (ruff, mypy, doctest, nbstripout, pytest: 9879 passed) is green locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix workflow issues: pin setup-uv to exact tag, skip docker push on PRs astral-sh/setup-uv publishes no bare v8 major tag, so pin v8.3.2. The docker workflow now only logs in and pushes outside pull requests; PR runs still build the image for validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix remaining CI failures: flaky rank assertion, uv run re-sync, Dockerfile apt package - test_torch_distributed: pool.map does not pin task index to the worker's distributed rank, so assert the rank is valid instead of equal to the task index - unit tests workflow: use 'uv run --no-sync' so pytest does not re-sync with default groups; also set default-groups = [] so the dev group (which includes CUDA-only cupy) is opt-in, matching the old poetry behavior - Dockerfile: drop software-properties-common, which no longer exists in Debian trixie and was unused Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

amrit110

CI is green after the rebase onto main (which now includes the uv/Python 3.11 migration in #768). The pc.day fix and added test look correct — thanks for the contribution!

driavysinus and others added 4 commits July 9, 2026 00:24

Fix datetime day filtering

89db2b5

[pre-commit.ci] Add auto fixes from pre-commit.com hooks

96dabf5

for more information, see https://pre-commit.ci

Restore unrelated notebook formatting

d0fdaa3

[pre-commit.ci] Add auto fixes from pre-commit.com hooks

0ea24c1

for more information, see https://pre-commit.ci

driavysinus marked this pull request as ready for review July 18, 2026 10:36

amrit110 mentioned this pull request Aug 4, 2026

Fix CI: remove nbqa hooks, migrate to Python 3.11 + uv, update vulnerable dependencies #768

Merged

Merge branch 'main' into fix/filter-datetime-day-component

166f871

amrit110 approved these changes Aug 4, 2026

View reviewed changes

amrit110 merged commit c88727c into VectorInstitute:main Aug 5, 2026
5 checks passed

Navigation Menu

Uh oh!

Fix datetime day filtering - #767

Fix datetime day filtering#767
amrit110 merged 5 commits into
VectorInstitute:mainfrom
driavysinus:fix/filter-datetime-day-component

driavysinus commented Jul 8, 2026 •

edited

Loading

Uh oh!

driavysinus commented Aug 3, 2026

Uh oh!

amrit110 commented Aug 4, 2026

Uh oh!

amrit110 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

driavysinus commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Validation

Uh oh!

driavysinus commented Aug 3, 2026

Uh oh!

amrit110 commented Aug 4, 2026

Uh oh!

amrit110 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

driavysinus commented Jul 8, 2026 •

edited

Loading