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.

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

Patch brakes the argument order #33

Open
astynax opened this issue Oct 10, 2024 · 1 comment
Open

Patch brakes the argument order #33

astynax opened this issue Oct 10, 2024 · 1 comment

Comments

@astynax
Copy link

astynax commented Oct 10, 2024

Hi!

This particular patch provides a subset of the original arguments. I can live with that theoretically, but it also swaps the positions of pk_items and updates. It took some time to find why my .update(id, {"a": "b"}) was broken 🙈 And the fact than my IDE (PyCharm) shows me hints about the original method and "go to definition" also jumps to the old one, doesn't help with such cases.

I understand that that "@patch"ing stuff was made for the sake of nbdev and it works in live environments. But such quirks with incompatibly changed method signatures make using of FastHTML in a Starlett's typical "hot reload" workflow pretty uncomfortable 😩

Maybe You can consider to make the args "keyword-only"? This will make the patching more robust.

@jph00
Copy link
Contributor

jph00 commented Dec 22, 2024

Probably the better solution here is to run py2pyi to generate .pyi files with the correct info. @pydanny we should discuss that sometime.

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

No branches or pull requests

2 participants