Support implementing functions in Python - Githubissues

caketop / python-starlark-go

🐍 Python bindings for starlark-go 🐍

https://python-starlark-go.readthedocs.io/

Apache License 2.0

23 stars 8 forks source link

Support implementing functions in Python #232

Closed brandonchinn178 closed 2 months ago

brandonchinn178 commented 3 months ago

Resolves #230

AlmogBaku commented 3 months ago

lgtm @jordemort

jordemort commented 3 months ago

Thanks, this looks pretty good. Two things:

The build fails on Python 3.7, 3.8, 3.9 because the exception message is not exactly what the test expects. I need to update my Python versions here and drop 3.7 because it's EOL, but 3.8 and 3.9 are still supported, so please loosen up the test so that it works there.
I'm not sure exactly where this capability should be mentioned in the docs, but it should be mentioned somewhere. Unfortunately, the bulk of the documentation is embedded in docstrings in starlark.c in a very ugly way.

brandonchinn178 commented 2 months ago

@jordemort How's that?

jordemort commented 2 months ago

This is looking good; the macOS builds are broken through no fault of yours. I fixed that, and dropped Python 3.7, and promoted Python 3.12 to production, and updated a few other dependencies. Would you mind rebasing on to / merging in my latest main? After that, if all the tests are passing, I think I am ready to merge it.

brandonchinn178 commented 2 months ago

@jordemort Should be good to go!

brandonchinn178 commented 2 months ago

@jordemort any updates?

jordemort commented 2 months ago

@brandonchinn178 I've merged it! There's a few other things I want to get to before doing another release, though, and I'm not sure when I'm going to get to them; if you or anyone else wants to run with these before I am able to make time, I am happy to review and merge the results:

It looks like I need to enable 2FA on test.pypi before it will let me do test uploads again
The embedded version of Starlark needs to be updated, but it's going to necessitate an API break, re: https://github.com/caketop/python-starlark-go/pull/204
There's some open issues related to adding support for structs, json functions, and other loadable stuff that I would like to include in a 2.0, if I'm going to have to break the API anyway

brandonchinn178 commented 2 months ago

Thanks @jordemort! Is there a reason why starlark-go "has" to be updated? I don't see why function support can't be released now in the 1.x series, decoupled from implementing all those 2.x features.

The only reason I'm asking is because it'd be nice to have a prebuilt wheel with function support, so I don't have to require users of my script to have golang installed to install python-starlark-go

jordemort commented 2 months ago

@brandonchinn178 I'll see if I can get an interim release out for you, I just feel bad pushing one with such an out-of-date release of starlark-go. Hopefully I'll get some time later in the week to chase the bats out of the CI.

brandonchinn178 commented 2 months ago

I'd really appreciate that! We don't mind the old version of starlark-go. Thanks a ton!