sizmailov / pybind11-stubgen

Generate stubs for python modules
Other
218 stars 44 forks source link

feat: ✨ automatically replace invalid enum expressions with corresponding valid expression & import #196

Closed ringohoffman closed 7 months ago

ringohoffman commented 7 months ago

Inspired by: https://github.com/sizmailov/pybind11-stubgen/issues/192

I think this feature supersedes the need for --enum-class-locations, though I did not remove it here. At least that is my goal. I want this project to be as easy for people to pick up as possible, and this was something I thought should be possible for pybind11_stubgen to do on behalf of the user.

We tweak RewritePybind11EnumValueRepr to delay report_error() for enum expressions. We remember all of the invalid enum expressions we have seen as we crawl through the module. When we find the definition of a value corresponding to an enum repr, we use it to fix invalid expressions we have seen or will see. If there are still invalid enum expressions left over, we report_error() during finalize().

sizmailov commented 7 months ago

This should eliminate the need for enum location specification for most use cases.

This PR introduces a bit of guessing to stub generation if the module has two enums with the same class and value names. Depending on the order of traversing the module, the resulting stubs could be different.

I suggest the following changes:

ringohoffman commented 7 months ago

@sizmailov are you planning to drop support for 3.7 at some point? It reached end of life status 5 months ago. I would like it if we did, because I want to use the assignment expression operator (:=). I think it can be great for readability if used correctly.

3.8's EOL is coming in only 10 months, also. numpy has already dropped support for it in preparation.

What are your thoughts about this?

sizmailov commented 7 months ago

I think we should aim for the same versions as pybind11, which is currently 3.6+. I don't remember exactly why I didn't support 3.6. Probably, it was the troubles of setting up the test for CI :disappointed:

So Python 3.7 support would stay for quite a while, for better or worse.

ringohoffman commented 7 months ago

@sizmailov Okay, I gave it my best effort.

  1. I moved the fixing to finalize().
  2. I only import module names.
  3. I check to see if there are multiple possible definitions for an invalid enum repr, and raise an error like:
    pybind11_stubgen - [  ERROR] In finalize : Enum member '<ConsoleForegroundColor.Magenta: 35>' could not be resolved; multiple identical definitions found in: 'demo._bindings.duplicate_enum', 'demo._bindings.enum'
  4. I added a new test that creates an ambiguous enum using ConsoleForegroundColorDuplicate. I had to update --ignore-invalid-expressions to incldue <ConsoleForegroundColor\\.Magenta: 35>.
  5. I needed to add a LoggerData override of finalize() to create a new layer so that I could call report_error() from finalize().
sizmailov commented 7 months ago

If you don't mind, I'll publish it as 2.4.1

ringohoffman commented 7 months ago

I don't mind! Thank you for reviewing my work!