Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added function to make kwargs json serializable in Benchmarks.evaluate #157

Merged
merged 2 commits into from
Mar 23, 2023
Merged

Added function to make kwargs json serializable in Benchmarks.evaluate #157

merged 2 commits into from
Mar 23, 2023

Conversation

robsdavis
Copy link
Contributor

@robsdavis robsdavis commented Mar 23, 2023 •
edited
Loading

Description

Added function to make kwargs json serializable in Benchmarks.evaluate, if a pathlib.Path object is passed to workspace. Closes #156

Affected Dependencies

None

Checklist

@robsdavis robsdavis added the bug Something isn't working label Mar 23, 2023
@robsdavis robsdavis linked an issue Mar 23, 2023 that may be closed by this pull request
kwargs hashing fails in benchmark.evaluate #156
Closed
@robsdavis robsdavis requested a review from bcebere March 23, 2023 12:37
bcebere
bcebere reviewed Mar 23, 2023
View reviewed changes
Copy link
Contributor

@bcebere bcebere left a comment

Choose a reason for hiding this comment

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

Do you have a test where this was failing with the previous version?

@robsdavis
Copy link
Contributor Author

I have just added to the test_benchmark_workspace_cache test to cover this case. This test passes with/fails without the new get_json_serializable_kwargs() call.

bcebere
bcebere approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@bcebere bcebere left a comment

Choose a reason for hiding this comment

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

LGTM!

@robsdavis robsdavis merged commit 0d48f3b into main Mar 23, 2023
@robsdavis robsdavis deleted the 156-kwargs-hashing-fails-in-benchmarkevaluate branch March 23, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcebere bcebere bcebere approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kwargs hashing fails in benchmark.evaluate
2 participants
@robsdavis @bcebere