Skip to content

[modular] Simplify logic and docstring handling #39185

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Cyrilvallez
Copy link
Member

@Cyrilvallez Cyrilvallez commented Jul 2, 2025

What does this PR do?

Simplify a lot replace_class_node logic (I wanted to do it from a long time ago as it was all bloated and super hard to read) and simplify the docstring handling at the same time.

Basically, the idea is now to use the parent docstring if there are no docstrings, or to use the docstring in modular if any. Previously, we were doing some kind of merging between the 2 docstrings, which was bugged (see a few places in the diff where there are arg explanations after examples or similar issues), inconsistent (i.e. adding a simple arg for a Config, while convenient, was always inconsistent because the checkpoints link was never updated correctly) and surprising (this is some kind of magic that contributors were very rarely understanding (which makes sense at it had a lot of issues as explained). Moreover, with auto_docstring now, we should not have to redefine the parameters explanations everytime, so it makes sense to relax docstring handling.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@Cyrilvallez Cyrilvallez changed the title [modular] Simplify logic [modular] Simplify logic and docstring handling Jul 2, 2025
Copy link
Contributor

github-actions bot commented Jul 2, 2025

[For maintainers] Suggested jobs to run (before merge)

run-slow: d_fine, diffllama, dinov2_with_registers, emu3, falcon_h1, gemma, gemma2, gemma3, gemma3n, glm4v, helium, instructblipvideo, internvl, mlcd, sam_hq, smolvlm

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