Skip to content

fix(save_df): include keyword names in the cache key - #311

Merged
tfardet merged 1 commit into
InseeFrLab:masterfrom
LemonCondor:fix-save-df-cache-key
Oct 14, 2026
Merged

tfardet merged 1 commit into
InseeFrLab:masterfrom
LemonCondor:fix-save-df-cache-key

Conversation

@LemonCondor

Copy link
Copy Markdown
Contributor

Fixes #310

save_df built the cache file name from keyword argument values only, joined without a separator, so different calls could share a cache file — e.g. get_series(id, endPeriod="2023-01") returned the data cached by an earlier get_series(id, startPeriod="2023-01").

Change

  • The key now uses name=value for keyword arguments, sorted by name (so keyword order doesn't create duplicate files), and parts are joined with a separator (\x1f) so positional boundaries can't collide (("AB", "C") vs ("A", "BC")).
  • Existing cache files get a new name, so they are simply re-downloaded once.

Tests

tests/utils/test_save_df.py (offline, uses a temporary cache folder):

  • startPeriod-like vs endPeriod-like keyword with the same value → distinct results (failed before)
  • positional arguments with the same concatenation → distinct results (failed before)
  • identical calls with keywords in a different order → one cache file

Also checked against the live API: after the fix, the example in #310 returns 397 rows (1990-01 .. 2023-01) as expected. tests/utils passes locally; black and flake8 pass with the project config.

The cache file name was built from keyword argument values only, joined
without a separator, so get_series(id, startPeriod="2023") and
get_series(id, endPeriod="2023") shared the same cached file.

Fixes InseeFrLab#310

Signed-off-by: Guillaume Marty <279286709+LemonCondor@users.noreply.github.com>
@tfardet

tfardet commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the fix, I'm trying to see whether @hadrilec can setup an environment so we can run the tests, will try to come back to you soon

@tfardet tfardet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I ran the test locally and everything looks good, thanks!

@tfardet
tfardet merged commit 4e34513 into InseeFrLab:master Oct 14, 2026
0 of 4 checks passed
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.

save_df cache key ignores keyword names: get_series(endPeriod=...) can return cached startPeriod data

2 participants