rewrite _process_parameter_type in auto_docstring.py to improve usability
Author: Yacklin
Pull request: yes

# What does this PR do?
When using kimi model, kimi developers used something like "bool|int" as type hint for parameters in function signature and unfortunately, due to python's nature, format of type hint like "bool|int" is not completely the same as "typing.Union[bool, int]" until python 3.14. One of the biggest differences is when accessing param.annotation.\_\_name\_\_, "bool|int" would raise error that has no attribute "\_\_name\_\_". This error bothers me a lot so i have to fix it. I found a beautiful way to fix it and its backward compatibility could be extended to 3.4 (I just tried it a little bit but you could try versions before 3.4).

Please be advised that param_name and func are unused in this rewrited version. I chose not to remove these two parameters just in case something bad would happen.
<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->



## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@Rocketknight1
(I accidentally deleted the previous PR, here is the new one)
<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker @Cyrilvallez
- vision models: @yonigozlan @molbap
- audio models: @eustlb @ebezzam @vasqu
- multimodal models: @zucchini-nlp
- graph models: @clefourrier

Library:

- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- continuous batching: @remi-or @ArthurZucker @McPatate
- pipelines: @Rocketknight1
- tokenizers: @ArthurZucker and @itazap
- trainer: @SunMarc
- attention: @vasqu @ArthurZucker @CyrilVallez
- model loading (from pretrained, etc): @CyrilVallez
- distributed: @3outeille @ArthurZucker
- CIs: @ydshieh

Integrations:

- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization: @SunMarc @MekkCyber
- kernels: @MekkCyber @drbh
- peft: @BenjaminBossan @githubnemo

Devices/Backends:

- AMD ROCm: @ivarflakstad
- Intel XPU: @IlyasMoutawwakil
- Ascend NPU: @ivarflakstad 

Documentation: @stevhliu

Research projects are not maintained and should be taken as is.

 -->


Comments:
Approved the doc build run! You should get a link to the docs with your PR in about 30 minutes - can you check that everything is working as expected?

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_42431). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

> Approved the doc build run! You should get a link to the docs with your PR in about 30 minutes - can you check that everything is working as expected?

i think yes. Could you please let me know if anything should be changed in my code for this PR? I tested this rewritten fuction several times as well. Also, i am a little bit new to git and github so sometimes i might mess up PR or commits. Sorry for that in advance. @Rocketknight1 

i am wondering why sometimes tests failed but sometimes didn't? And my changes have nothing to do with these failures

> Approved the doc build run! You should get a link to the docs with your PR in about 30 minutes - can you check that everything is working as expected?

Okay i do find something changed, which i would prefer actually.
current one:
<img width="842" height="106" alt="image" src="https://github.com/user-attachments/assets/1be4c41a-f54b-4e7b-96fa-e5e9f3fe0858" />
updated one:
<img width="860" height="111" alt="image" src="https://github.com/user-attachments/assets/0091d10c-e511-4999-b559-1f25e4aa500a" />

Might be a little bit verbose but i think it's okay

Maybe i should provide more details about this PR.
When using kimi 48B instruct model, the decorator \@auto_docstring would be applied to some functions in modelling_kimi like this:
<img width="532" height="342" alt="image" src="https://github.com/user-attachments/assets/fb5a13d1-73e2-436a-82d5-d347b0a6cd30" />

But, auto_docstring would call _process_parameter_type to go through every single parameter's type hint to call this type hint's annotation.\_\_name\_\_. But before python 3.14, bool|int is not the same as Union[bool, int]. It means type hint like "bool|hint" has no attribute \_\_name\_\_. Then _process_parameter_type would raise errors, which prevents users from using kimi 48B instruct models.
<img width="856" height="39" alt="image" src="https://github.com/user-attachments/assets/a348e048-1426-4fdc-b015-7d84bea128c2" />

Btw, the downstream users of kimi 48B noticed this problem then they changed it but it seems that they didn't notify kimi this problem:

<img width="488" height="251" alt="image" src="https://github.com/user-attachments/assets/c64acecf-8095-4be9-afd6-d0a8768f723c" />
(coming from modelling_kimi.py of cerebras/Kimi-Linear-REAP-35B-A3B-Instruct)

My PR could solve this problem. Otherwise, python 3.14 is needed to use kimi 48B instruct model and i don't think transformers users could switch to python 3.14 easily
@Rocketknight1 

> Hey @Yacklin, I took a look at the code! I think the bug you're fixing is real, and I think the cause is that we do lots of hacky accesses to things like `annotation.__name__`, like you said. I don't like `if ":" in parameter_str` though - doing these kinds of string manipulations also feels very hacky. It's likely to create bugs in future if anything changes with typing, or if anyone writes an unusual type hint.
> 
> Is there a clean way to correctly resolve all type hints into some kind of canonical form, and then just operate on that, rather than relying on string representations and attribute accesses?

Hi @Rocketknight1 , string manipulation is unavoidable in this function because this function would return something like tuple[str, bool]. But i could try to write an implmentation to minimize string manipulation.

Hi @Yacklin, some manipulation is unavoidable at the end, but I think you should always be able to get the type hint from `param.annotation` rather than needing to use `str(param)`. What we really want to do is just grab that annotation in the safest and most Pythonic way possible and turn it into a "canonical" form regardless of whether it was written as `Optional[Union[str, bool]]` or `str | bool | None`. I don't think flattening it to a string is the safest way to do that, and will probably get very messy for nested types!

> Hi @Yacklin, some manipulation is unavoidable at the end, but I think you should always be able to get the type hint from `param.annotation` rather than needing to use `str(param)`. What we really want to do is just grab that annotation in the safest and most Pythonic way possible and turn it into a "canonical" form regardless of whether it was written as `Optional[Union[str, bool]]` or `str | bool | None`. I don't think flattening it to a string is the safest way to do that, and will probably get very messy for nested types!

Hi @Rocketknight1 ,

This situation is more complex than i thought. Please read my comments and implementations carefully. Let me know what you think.

```py

import inspect
import types
import typing
from typing import get_origin, Union, get_args, Optional

def _process_parameter_type(param, param_name, func):
    """
    Process and format a parameter's type annotation.

    Args:
        param (`inspect.Parameter`): The parameter from the function signature
        param_name (`str`): The name of the parameter
        func (`function`): The function the parameter belongs to
    """
    """
    the minimum version of python that transformers supports is 3.9.0.
    but this is the source of problem.

    if user who is using python 3.9.x tried to access models that are using features coming from types.UnionType, like int|float, which is introduced in python 3.10,
    error would be raised (even before this function is executed). Python developers came up with an idea that would squash int|float into string "int|float", using 'from __future__ import annotations'.
    But this is something models developers should care, transformers developers could only warn models developers to solve this issue in their own model script.

    the best way to solve this issue is to level up the minimum python version from 3.9 to 3.10

    if user is using python 3.9, this error would be raised before this function is hit.

    unfortunately, most models developers didn't pay attention to it.

    If python 3.9 users tried to access models that are using features coming from types.UnionType, like int|float, which is introduced in python 3.10,
    users have to contact models developers to support backward-compatibility. Transformers team has nothing to do with it, unless upgrading min version from
    3.9 to 3.10, through voting or something.
    """

    # this PR only takes python 3.10 into consideration, unfortunately.

    param_annotation = param.annotation
    print(param_annotation)
    default_value = param.default
    origin = get_origin(param_annotation)
    args = get_args(param_annotation)
    optional_flag = False
    type_hint_str = ""

    # Optional[sth], under the hood is equivalent to Union[sth, None]
    # if the type hint is None, I mean something like age:None, it would be considered optional, very few developers would write it though.
    # if the type hint is Optional[None] or Union[None], it would be considered optional, very few developers would write it though.
    if (
        (origin is typing.Union and type(None) in args)
        or default_value is not inspect._empty
        or (origin is types.UnionType and type(None) in args)
        or param_annotation is types.NoneType
        or param_annotation is None
    ):
        optional_flag = True

    """
    okay here is another problem, to get the string representation of type hint int, param.annotation.__name__ has to be accessed BUT
    types.UnionType, like int|str, before python 3.14 has no attribute __name__, to get the string representation of int|str, access param.annotation ONLY.
    """
    param_str: str = str(param)
    if param_annotation is not inspect._empty:
        if "=" in param_str:
            type_hint_str = (
                param_str.split(":", maxsplit=1)[1].split("=", maxsplit=1)[0].strip()
            )
        else:
            type_hint_str = param_str.split(":", maxsplit=1)[1].strip()

    return type_hint_str, optional_flag
```

To make sure python 3.9 users could use features like int|str, which is introduced in python 3.10 correctly, what IDE and PEP suggests is to use "from \_\_future\_\_ import annotations", within from \_\_future\_\_ import annotations, you could see it clearly that type hints would all be converted to strings at Runtime.
I don't mean to but this is what python developers did.
<img width="1022" height="296" alt="image" src="https://github.com/user-attachments/assets/b40eb51c-dc5e-42d5-aaa8-b4a3c735078c" />


I can confirm that it fixed the issue and I was able to get it working, thank you.

> I can confirm that it fixed the issue and I was able to get it working, thank you.

But i need to make reviewers happy about my changes, which is harder than writing a PR

man you're lucky i'm on watch and isn't sgugger anymore or he would have had you guillotined for this level of sass :sweat_smile: 

Also, quick comment about your implementation: Python 3.9 is now [end-of-life](https://devguide.python.org/versions/). As a result, we're moving to a minimum supported version of 3.10, and it's not necessary to consider Py3.9 compatibility in new code. It's fine for your PR to assume that everyone will be running 3.10 or newer

> man you're lucky i'm on watch and isn't sgugger anymore or he would have had you guillotined for this level of sass 😅

Oh Thanks to let me know hahaha

> Also, quick comment about your implementation: Python 3.9 is now [end-of-life](https://devguide.python.org/versions/). As a result, we're moving to a minimum supported version of 3.10, and it's not necessary to consider Py3.9 compatibility in new code. It's fine for your PR to assume that everyone will be running 3.10 or newer

If you are happy with the implementation i have in the comment, i would put it in my branch later. Thanks to let me know that 3.9 is end of life.

@Rocketknight1 I think it is good to go. I reviewed the docs for this PR and it looks good.
Also, some tests sometimes failed and sometimes didn't, but i believe you are fixing it right now.

this failed test shows up again....
<img width="1300" height="55" alt="image" src="https://github.com/user-attachments/assets/2d952261-43a4-494e-b13f-5866f1565449" />


@Rocketknight1 Remember to review my new code btw

@Rocketknight1 this is your last review for this PR i promise!!!

Man you changed my implementation significantly that i can't even understand lol

@Rocketknight1 , i just tested your implementation and found it weird and wrong. Please revert your changes
<img width="388" height="272" alt="image" src="https://github.com/user-attachments/assets/28cdfe40-e5ce-4322-9a7e-288e2d5dbd8e" />


@Rocketknight1 Man why don't you build upon my latest implementation? No need to build it again, if you read all my comments carefully!

@Yacklin you gotta calm down lol. I'm the watcher for this entire repo so I have like 50 issues a day and this isn't a priority one. I'll make a fix and hopefully improve the docstring generation in general, but you need to let go of the laptop and touch grass while I do instead of pinging me every two hours!

> @Yacklin you gotta calm down lol. I'm the watcher for this entire repo so I have like 50 issues a day and this isn't a priority one. I'll make a fix and hopefully improve the docstring generation in general, but you need to let go of the laptop and touch grass while I do instead of pinging me every two hours!

Sorry i just felt surprised by your implementation but do let me know if you ran into troubles while updating this PR

cc @yacklin it seems to be giving the same outputs as the old system now, but cleaned up a lot, and working correctly for `|` union types (I made a test in GPT-J for that, but we can revert before merging).

Want to take a look and tell me if you're happy with it, and then I'll ping a core maintainer if so?

Also, if the parameter has default value, it should be considered optional. It seems that your implementation didn't include this. I am wondering whether this function is supposed to consider default value or not?

@Yacklin I think that behaviour for type hints like `age: None` is fine - that type hint should almost never appear, since it indicates that the only acceptable argument value is `None`. The much more common case is something like `int | none` or `Union[int, None]` or `Optional[int]`, which all mean the same thing, and I think it handles that correctly, right?

> @Yacklin I think that behaviour for type hints like `age: None` is fine - that type hint should almost never appear, since it indicates that the only acceptable argument value is `None`. The much more common case is something like `int | none` or `Union[int, None]` or `Optional[int]`, which all mean the same thing, and I think it handles that correctly, right?

@Rocketknight1 , A quick question: Should default value be considered in this function? I mean if there is a default value for a parameter, this parameter should be optional. If other functions are designed to handle this, simply ignore what i highlighted here.
I just tested it and it seems that the my idea of implementation is slightly different from yours. Since you are a watcher, i will take yours.

maybe we could revert changes on gptj since this PR doesn't do anything with it.

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

run-slow: gptj

View the CircleCI Test Summary for this PR:

https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=42431&sha=0f3518

Yeah, those were mostly for testing. Will revert the GPT-J changes, keep the rest and merge!

I just realized that when the type hint of a parameter is None, the only acceptable argument is None for this parameter. And it should be marked as non-optional. But since nobody would annotate a parameter with None, the if statement i wrote to handle None is useless. You could remove that line, if necessary. Anyway, thanks for editing!

@Rocketknight1 , also, the way this function handles default value seems to be not perfect (my bad). If a parameter has no type hint but has default value, it would be considered non-optional. Maybe it would introduce some problems in the future.