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

bug: Pydantic dataclasses with extra args break parsing of the function signature #303

Open
Vodes opened this issue Jul 6, 2024 · 9 comments
Assignees
Labels
feature New feature or request fund Issue priority can be boosted griffe extension Can be solved with a Griffe extension insiders Candidate for Insiders

Comments

@Vodes
Copy link

Vodes commented Jul 6, 2024

Description of the bug

Griffe fails to parse the class init signature for pydantic dataclasses that allow extra kwargs.
The result is missing types in the documentation and everything marked as required.

allow_extra = ConfigDict(extra="allow", str_strip_whitespace=True, allow_inf_nan=False, arbitrary_types_allowed=True)


@dataclass(config=allow_extra)
class FDK_AAC(Encoder):
    """
    Uses the libFDK implementation in ffmpeg to encode audio to AAC.
    It's strongly recommended to use qAAC if you're on windows because its straight up the best AAC encoder.

    :param bitrate_mode:        Any int value from 0 - 5
                                0 will be CBR and using the bitrate below, 1 - 5 are true VBR modes
                                See https://wiki.hydrogenaud.io/index.php?title=Fraunhofer_FDK_AAC#Bitrate_Modes

    :param bitrate:             Any int value representing kbps
    :param cutoff:              Hard frequency cutoff. 20 kHz is a good default and setting it to 0 will let it choose automatically.
    :param preprocess:          Any amount of preprocessors to run before passing it to the encoder.
    :param use_binary:          Whether to use the fdkaac encoder binary or ffmpeg.
                                If you don't have ffmpeg compiled with libfdk it will try to fall back to the binary.
    :param append:              Any other args one might pass to the encoder
    :param output:              Custom output. Can be a dir or a file.
                                Do not specify an extension unless you know what you're doing.
    """

    bitrate_mode: int = 5
    bitrate: int = 256
    cutoff: int = 20000
    preprocess: Preprocessor | Sequence[Preprocessor] | None = None
    use_binary: bool = False
    append: str = ""
    output: PathLike | None = None

image

To Reproduce

Install the package with the dynamic-dataclasses branch.

pip install git+https://github.com/Jaded-Encoding-Thaumaturgy/muxtools.git@dynamic-dataclasses

And then either use the docs repo itself like

git clone https://github.com/Vodes/muxtools-doc.git
cd muxtools-doc
pip install mkdocs-material mkdocstrings[python]
pip install --no-deps git+https://github.com/Jaded-Encoding-Thaumaturgy/vs-muxtools.git
pip install --no-deps git+https://github.com/Vodes/vodesfunc.git
mkdocs serve

http://localhost:8000/muxtools/audio/encoders/ will then show the issue.

or just use a blanket docs setup that just embeds the encoders file like this

Full traceback

Full traceback
WARNING -  griffe: C:\Users\Alex\AppData\Roaming\Python\Python312\site-packages\muxtools\audio\encoders.py:274: Parameter 'bitrate_mode' does not appear in the function signature
WARNING -  griffe: C:\Users\Alex\AppData\Roaming\Python\Python312\site-packages\muxtools\audio\encoders.py:274: No matching parameter for 'bitrate_mode'
WARNING -  griffe: C:\Users\Alex\AppData\Roaming\Python\Python312\site-packages\muxtools\audio\encoders.py:274: Parameter 'bitrate' does not appear in the function signature
WARNING -  griffe: C:\Users\Alex\AppData\Roaming\Python\Python312\site-packages\muxtools\audio\encoders.py:274: No matching parameter for 'bitrate'
WARNING -  griffe: C:\Users\Alex\AppData\Roaming\Python\Python312\site-packages\muxtools\audio\encoders.py:274: Parameter 'cutoff' does not appear in the function signature
WARNING -  griffe: C:\Users\Alex\AppData\Roaming\Python\Python312\site-packages\muxtools\audio\encoders.py:274: No matching parameter for 'cutoff'
...

Expected behavior

Properly parse types in the signature.
Using a vanilla python dataclass results in the wanted behavior.

image

Environment information

- __System__: Windows-11-10.0.22631-SP0
- __Python__: cpython 3.12.3 (Z:\Program Files\Python312\python.exe)
- __Environment variables__:
- __Installed packages__:
  - `griffe` v0.47.0

Boost priority

  • Boost priority in our backlog through Polar.sh. Higher pledge, higher priority.
  • Minimum pledge by user/organization is $5, minimum amount for boost is $30.
  • View all issues with pledges.
  • We receive the funds once the issue is completed and confirmed by you.
  • Features with the insiders label are released to sponsors first, and tied to a funding goal.
Fund with Polar
@Vodes Vodes added the unconfirmed This bug was not reproduced yet label Jul 6, 2024
@pawamoy
Copy link
Member

pawamoy commented Jul 6, 2024

Hi @Vodes, thanks for the report.

For future reference and ease of reading, could you add a snippet showing your Pydantic dataclass declaration?

@Vodes
Copy link
Author

Vodes commented Jul 6, 2024

Yeah no problem. The class that is also used in the screenshots is defined right here.
The config being used is in this seperate file to not clutter up everything else.

@pawamoy
Copy link
Member

pawamoy commented Jul 6, 2024

Thanks, let me update the issue body with these.

@pawamoy
Copy link
Member

pawamoy commented Jul 6, 2024

I suppose we will have to somehow duplicate the code of our built-in dataclasses extension into the sponsors-only pydantic extension, with additional logic since Pydantic's dataclasses allow a few more things. I'll mark this as feature request instead of bug!

@pawamoy pawamoy added feature New feature or request griffe extension Can be solved with a Griffe extension insiders Candidate for Insiders and removed unconfirmed This bug was not reproduced yet labels Jul 6, 2024
@Vodes
Copy link
Author

Vodes commented Jul 6, 2024

So I understand, this is out of scope for griffe without the extension? I'm not really interested in pydantic stuff outside of the dataclass and I only really care about the actual attributes being presented properly.

@pawamoy
Copy link
Member

pawamoy commented Jul 6, 2024

Yep, what we incorporate in Griffe is support for the standard lib and the Python language itself. Dataclasses, even though standard, are a bit special (and quite hard to handle statically) so they are supported through a built-in extension.

Pydantic models and dataclasses are third-party, so they are handled through an external extension.

If you don't want to become a sponsor 😜 and rely on the Pydantic extension, you could try to force dynamic analysis on such dataclasses (which could help for the issue here). I'll add an example on how to do that in the docs 🙂

@Vodes
Copy link
Author

Vodes commented Jul 27, 2024

Hey so I just noticed that there were some docs related commits lately but would you mind pointing me in the right direction of what I should enable to make this work?

@Vodes
Copy link
Author

Vodes commented Oct 12, 2024

I didn't want to bump this but I assume now that the extension is public this isn't really a concern anymore. Feel free to close if so.

@pawamoy
Copy link
Member

pawamoy commented Oct 13, 2024

Sorry, missed your previous reply. By the way, feel free to ping me if I don't answer in the future (that's what my GitHub status say 😄!).

The griffe-pydantic extension just became public indeed (thanks to my awesome sponsors 🚀), but it doesn't have any kind of support for Pydantic's dataclasses yet, so I believe we can keep this open.

@pawamoy pawamoy added the fund Issue priority can be boosted label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request fund Issue priority can be boosted griffe extension Can be solved with a Griffe extension insiders Candidate for Insiders
Projects
None yet
Development

No branches or pull requests

2 participants