feat(serializer): add support for FunctionType serialization #823
Conversation
* add FunctionType serialization in amber and json extensions * add tests
|
@noahnu any idea why the Benchmark workflow is failing? Doesn't seem to be an issue with the PR changes. |
|
You can ignore the benchmark. It always fails for external contributors. There are credentials that don't get exposed. I'll try to review this pull request soon, thank you for the contribution! |
| if isinstance(data, FunctionType): | ||
| return ( | ||
| f"<{FunctionType.__name__} " | ||
| f"'{data.__qualname__}{str(inspect.signature(data))}'>" |
There was a problem hiding this comment.
Serialization nitpick
| f"'{data.__qualname__}{str(inspect.signature(data))}'>" | |
| f"{data.__qualname__}{str(inspect.signature(data))}>" |
Result
"<function function_to_test(var1, var2='test_val', var3: str = 'test_val2', *, kwvar1, kwvar2='some_val') -> str>"There was a problem hiding this comment.
I'm OK with this, but this would be inconsistent with class, which surrounds in single quotes. However, I think because of the single quotes in the inspect signature, I'm going to go ahead with this change.
|
Thanks for the review! I've updated with the suggested changes. |
|
Snapshot needs to be updated. |
Yeah I had left some testing code in the amber file by accident, fixed. Thanks! |
|
Caught the formatting issue, but not fast enough. Sorry about that, used to format on save and I haven't setup vscode for this repo yet. |
|
@all-contributors add @ManiacDC for code |
|
I've put up a pull request to add @ManiacDC! 🎉 |
# [4.6.0](v4.5.0...v4.6.0) (2023-10-24) ### Features * **serializer:** add support for FunctionType serialization ([#823](#823)) ([f3a454a](f3a454a))
|
🎉 This PR is included in version 4.6.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
Thank you! |
Description
add FunctionType serialization in amber and json extensions
Related Issues
Checklist
Additional Comments
I used inspect as I didn't want to re-invent the wheel. Is this satisfactory?
Also of minor concern - this changes the format of existing FunctionType serialization. I don't think this realistically causes a problem as the prior serialization would change every time it ran anyway.