Skip to content

Conversation

ISOR3X
Copy link
Contributor

@ISOR3X ISOR3X commented Apr 27, 2025

Added a construct_signature function to the Function class. This closely follows the way the jinja template works for the mkdocstrings python handler

The reason behind this PR is because I want to try to integrate python api docs into a vitepress website, but I also noticed both griffe2md and mkdocstrings/python/ have similar logic for building a function signature. Perhaps this code could replace that.

REF: #376

@ISOR3X
Copy link
Contributor Author

ISOR3X commented Apr 27, 2025

Those errors do not seem to come from my code so I am not sure what to do. This is one of my first time contributing to open sources projects to I might need some guidance here.

@pawamoy
Copy link
Member

pawamoy commented Apr 30, 2025

Thanks a lot for your PR @ISOR3X!

The errors I see in the CI logs do seem to come from your changes: you removed the Union import in src/_griffe/models.py and Ruff is complaining.

You also added spacing changes that are irrelevant to the feature here (added tabulations), could you revert these? You can run make format to format the code according to the project rules.

@ISOR3X
Copy link
Contributor Author

ISOR3X commented May 1, 2025

Fixed the formatting but the CI still seems to fail...

Also make format doesn't work for me, but I did noticed my IDE (PyCharm) automatically made a venv when opening the project, do you know something about this?

@ISOR3X
Copy link
Contributor Author

ISOR3X commented Jul 21, 2025

I was worried that this was forgotten, glad to see it's not!

Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot!

@pawamoy pawamoy merged commit 8ef1486 into mkdocstrings:main Jul 21, 2025
24 of 26 checks passed
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

Successfully merging this pull request may close these issues.

2 participants