Skip to content

[cleanup] Avoid importing pandas at ccflow import time - #272

Merged
timkpaine merged 1 commit into
mainfrom
cleanup/lazy-numpy-pandas-tokenize
Sep 21, 2026
Merged

timkpaine merged 1 commit into
mainfrom
cleanup/lazy-numpy-pandas-tokenize

Conversation

@ptomecek

@ptomecek ptomecek commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

import ccflow pulled in pandas unconditionally. The only pandas-specific handler in the tokenizer is for pd.Timestamp, and registering it meant importing pandas at module load, so every user paid that cost whether or not they ever tokenized a pandas object.

pd.Timestamp subclasses datetime, so it already reaches the datetime handler. The only thing the dedicated registration added was a distinct tag, so that a Timestamp and an equal plain datetime don't produce the same token. Matching on the type's name gives the same result without the import.

Tokens are unchanged, so existing cache keys stay valid.

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    1 files  ±0      1 suites  ±0   3m 3s ⏱️ +29s
1 360 tests ±0  1 358 ✅ ±0  2 💤 ±0  0 ❌ ±0 
1 366 runs  ±0  1 364 ✅ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit ad447ed. ± Comparison against base commit 83bca9a.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.58%. Comparing base (1e18c52) to head (ad447ed).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
- Coverage   93.58%   93.58%   -0.01%     
==========================================
  Files         176      176              
  Lines       20590    20586       -4     
  Branches     1358     1359       +1     
==========================================
- Hits        19269    19265       -4     
  Misses       1048     1048              
  Partials      273      273              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ptomecek
ptomecek force-pushed the cleanup/lazy-numpy-pandas-tokenize branch from b2e0f2d to c5f3949 Compare September 21, 2026 18:43
@ptomecek
ptomecek marked this pull request as draft September 21, 2026 18:52
@ptomecek
ptomecek force-pushed the cleanup/lazy-numpy-pandas-tokenize branch from c5f3949 to 0e55267 Compare September 21, 2026 19:35
@ptomecek ptomecek changed the title [cleanup] Lazily register numpy/pandas tokenize handlers [cleanup] Avoid importing pandas at ccflow import time Sep 21, 2026
@ptomecek
ptomecek marked this pull request as ready for review September 21, 2026 19:47
normalize_token registered its only pandas handler by dispatching on
pd.Timestamp, which required `import pandas` at module load and so made
every `import ccflow` pay for pandas whether or not the caller ever
tokenizes one.

That handler's body is identical to the datetime one apart from its tag,
and pandas was only ever needed as a dispatch key, so identify Timestamp
by name inside the existing datetime handler instead. Subclasses are
still matched by walking the MRO, and plain datetimes cost one extra
identity check.

numpy is deliberately left alone: ccflow.exttypes.pydantic_numpy already
imports it eagerly, so deferring its handlers would have no effect today.

Token values are unchanged, so existing cache keys remain valid.

Signed-off-by: Pascal Tomecek <pascal.tomecek@cubistsystematic.com>
@ptomecek
ptomecek force-pushed the cleanup/lazy-numpy-pandas-tokenize branch from 0e55267 to ad447ed Compare September 21, 2026 19:48
@timkpaine
timkpaine merged commit e65ff57 into main Sep 21, 2026
20 checks passed
@timkpaine
timkpaine deleted the cleanup/lazy-numpy-pandas-tokenize branch September 21, 2026 21:43
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.

2 participants