Skip to content

Fix benchmark harness recording zero timings under pandas 3 - #541

Open
abelianbee wants to merge 2 commits into
Quantco:mainfrom
abelianbee:fix-benchmark-chained-assignment
Open

abelianbee wants to merge 2 commits into
Quantco:mainfrom
abelianbee:fix-benchmark-chained-assignment

Conversation

@abelianbee

Copy link
Copy Markdown

The benchmark harness writes results with times["time"].iloc[i] = ..., which is a chained assignment. Under pandas 3 copy-on-write that is rejected with ChainedAssignmentError and the value is dropped, so every timing and memory figure the harness produced was its initial value. Switched to .loc.

The new test fails on main and passes with the fix.

Checklist

  • Added a CHANGELOG.rst entry

times["time"].iloc[i] = ... is a chained assignment. pandas 3 rejects it with
ChainedAssignmentError and discards the value, so every timing and memory
figure the harness produced was its initial value. Use .loc instead.

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.

Thank you, tiny nit aside looks good.

bench_memory=False,
)
assert times["time"].notna().all()
assert (times["time"] > 0).all()

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.

Suggested change
assert (times["time"] > 0).all()
assert times["memory"].notna().all()

Nit: this is a slightly more direct test

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