Should stubs contain default values?

[hauntsaninja]

This has come up recently, for similar reasons as #4881 and discussed a little there

From @gramster

an advantage of including default values is that tools like pyright or pylance will use stubs (if available) for the signatures they show to users when hovering over a function call, and users want to see the default values. I'd like to understand your point of view though as to why you would prefer them removed?

I'm overall fine with this, though I'd like to see the following:

• we restrict it to simple literal values (e.g. anything that can go into a Literal)
• we can still use ... at author's discretion (useful in overloads or cases where value doesn't actually matter or to avoid awful x: Literal["x"] = "x" things)
• we update e.g. stubtest to verify stuff

The case for:

• lets IDEs provide more information to users

The case against:

• most things are longer than ... and terseness may be more valuable than the additional information
• it's a thing that needs doing and maintaining
• if we allow complicated default values there are more costs

A similar thing could apply to constants.

[JelleZijlstra]

I agree, a couple of small comments:

• we restrict it to simple literal values (e.g. anything that can go into a Literal)

Other values that we could accept would include floats and types (e.g., for params like dict_factory: Callable[..., ...] = dict).

• we can still use ... at author's discretion (useful in overloads or cases where value doesn't actually matter or to avoid awful x: Literal["x"] = "x" things)
• we update e.g. stubtest to verify stuff

It should be easy for stubtest to check defaults for correctness on Python functions (it's in the signature). For C functions, it's not necessarily possible to infer the default though, and indeed there may not be a default in a useful sense.

What other tools would need updating? I think we'd have to change flake8-pyi and hopefully nothing else.

[srittau]

It would be useful - but not necessary - to update stubgen as well.

[AlexWaygood]

I like the idea of adding simple default values to stubs. I agree that it's very useful in IDEs, and I think it'll be easier to maintain than adding docstrings to stubs.

The = ... convention that we currently use is nice because it keeps stubs as concise as possible. But... it looks really weird to people who aren't used to it. I think it's nice for stubs to use the same syntax and conventions as normal Python wherever possible, and I remember finding the = ...s everywhere really strange and alien when I first started reading through typeshed's stubs as a new contributor.

What other tools would need updating? I think we'd have to change flake8-pyi and hopefully nothing else.

Pyright also has a lint that enforces only using ... as a default in stub files (ironically, since it's one of the IDEs that would benefit most from this change). We might be able to easily switch that off in our pyrightconfig file -- I'm not sure if it's bundled together with other stub-related lints or if it's a dedicated error code.

[sobolevn]

I have some concerns about it.

First of all, this will require more branching just for the sake of default values if it is different for different python versions.

Or we can show incorrect default values for this specific python version.

Secondly, default values are not a part of the signature. So, they can be changed in bugfix releases. It will make it much harder to maintain.

Next, stubs are even harder to maintain with default values.

Moreover, there are quite ugly defaults values like _sentinel = object() or others. Not sure we want this.

I think that IDEs can just use inspect to get the function and its defaults. It is much easier and it will always show correct results.

[AlexWaygood]

Moreover, there are quite ugly defaults values like _sentinel = object() or others. Not sure we want this.

I like @hauntsaninja's idea of only including "simple" defaults, e.g. "anything that's valid to go in a Literal slice". We can continue to use ... for ugly or complex defaults.

[srittau]

I'll honestly expect 95% of all default values to be None anyway.

[JelleZijlstra]

First of all, this will require more branching just for the sake of default values if it is different for different python versions.

That will happen sometimes, but my sense is that it's not common.

Secondly, default values are not a part of the signature. So, they can be changed in bugfix releases. It will make it much harder to maintain.

In theory, sure, but I'd be surprised if that happens with any frequency.

If maintainability concerns like these do come up frequently, we can revisit our decision, but I'd be surprised if it is a major issue. The stdlib doesn't change its defaults that much, and for third-party libraries we only support one version at a time.

[gramster]

I think that IDEs can just use inspect to get the function and its defaults. It is much easier and it will always show correct results. Easy in Jupyter with a running kernel; not as simple as it sounds in other IDEs that may not be written in Python, need to be responsive, and don't want to do a bunch of indexing up front.

…

On Thu, Oct 27, 2022 at 8:22 AM Jelle Zijlstra ***@***.***> wrote: First of all, this will require more branching just for the sake of default values if it is different for different python versions. That will happen sometimes, but my sense is that it's not common. Secondly, default values are not a part of the signature. So, they can be changed in bugfix releases. It will make it much harder to maintain. In theory, sure, but I'd be surprised if that happens with any frequency. If maintainability concerns like these do come up frequently, we can revisit our decision, but I'd be surprised if it is a major issue. The stdlib doesn't change its defaults that much, and for third-party libraries we only support one version at a time. — Reply to this email directly, view it on GitHub <#8988 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVCPCAEIXNELUUVHAVZ2WLWFKM47ANCNFSM6AAAAAAROQJKTU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jpe]

IDEs will also probably want docstrings.

[JelleZijlstra]

Docstrings provide significantly more maintainability challenges. It's worth discussing whether it's worth changing our stance on docstrings too, but let's do that in a separate issue.

[AlexWaygood]

IDEs will also probably want docstrings.

Please don't discuss docstrings here. We have the following issue for that discussion:

• Should stubs contain docstrings? #4881

[Avasam]

As a user of both libraries typed through Typeshed and inline-typed ones, I've noticed that's information lost in these third-party stubs, which is unfortunate. I really like when my IDE shows me default values of function parameters.

A similar thing could apply to constants.

I share the same feeling for constants. (Relates to #7258 ?)

Similarly to my comment here (#4881 (comment)) and as gramster mentionned: Is this something editors could inspect when they come across =... ? (ie: fallback to the source python code, if available).

What's a good compromise between maintainability and not hiding information that would otherwise be available?

On the idea of only doing it for simple types: stubtest could help validate and maintain accuracy. By knowing when a default is both known and considered "simple".

[JelleZijlstra]

It seems like there is agreement among the maintainers here, so I think we can move forward with changing our policy and allowing defaults. I wrote a little tool to help infer defaults from the runtime: https://github.com/JelleZijlstra/stubdefaulter. I can also work on adding support to stubtest for checking that defaults are right.

[Avasam]

https://github.com/microsoft/pyright/releases/tag/1.1.284

Behavior Change: Removed diagnostic check that detects a non-ellipsis default value in a stub file. This check was based on a now-outdated best practice. We now recommend that stubs include default values for better usability in language servers.

[AlexWaygood]

We've had great progress on this recently:

• Pyright and flake8-pyi have both removed their lints forbidding stubs to have defaults
• @JelleZijlstra's written a codemod to automatically insert defaults into stubs
• @JelleZijlstra recently contributed a fix to pytype to prevent a crash if a stub has a string-literal as a default value
• @JelleZijlstra contributed an enhancement to stubtest so that it verifies defaults in stubs are correct.
• stdlib: add defaults #9501 by @JelleZijlstra adds (some) defaults to the stdlib.

TODO, if anybody feels like taking these on:

• These docs need to be updated:

  typeshed/CONTRIBUTING.md

  Lines 450 to 453 in 2b9f200

  	Stub files should only contain information necessary for the type
  	checker, and leave out unnecessary detail:
  	* for arguments with a default, use `...` instead of the actual
  	default;

  . It's no longer true that "stub files should only contain information necessary for the type checker" -- they're also useful for IDEs. And, obviously, it's no longer true that default values should be omitted in all circumstances. Edit: I've filed Rework CONTRIBUTING.md: simple parameter defaults are now accepted and encouraged #9665 for this.
• Mypy's stubgen tool should be updated so that it no longer omits default values in all circumstances when generating stubs.
• Help Jelle out with improving his codemod tool: see https://github.com/JelleZijlstra/stubdefaulter/issues and https://github.com/JelleZijlstra/stubdefaulter#limitationstodos
• After the stdlib PR is merged, we can think about adding defaults to third-party stubs. It might be wise for us to wait for a mypy release that includes python/mypy@a9c62c5 before doing that, however.

[Avasam]

Do you wanna use this issue to track running the codemod on existing third-party stubs?

[JelleZijlstra]

My tool has a couple of problems, opened issues in https://github.com/JelleZijlstra/stubdefaulter. Hopefully I'll have time to work on those soon.

[AlexWaygood]

#9501 added a couple of defaults that are set to sys.maxsize at runtime, e.g.

typeshed/stdlib/collections/__init__.pyi

Line 166 in 7e40d70

def count(self, sub: str | UserString, start: int = 0, end: int = 9223372036854775807) -> int: ...

The runtime source for this method is here: https://github.com/python/cpython/blob/3847a6c64b96bb2cb93be394a590d4df2c35e876/Lib/collections/__init__.py#L1439

The precise value of sys.maxsize will vary depending on what platform you're running Python on, so I think we should probably revert the changes where the default is set to sys.maxsize at runtime.

We could also consider adding a lint to flake8-pyi that forbids numeric defaults >6 digits (we can bikeshed the precise number of digits we want to permit) — thoughts?

[AlexWaygood]

Alternatively we could add some special-casing to flake8-pyi around sys.maxsize specifically, so that this is allowed in a stub:

def count(self, sub: str | UserString, start: int = 0, end: int = sys.maxsize) -> int: ...

[JelleZijlstra]

Sorry for missing that! For now let's just remove those defaults.

sys.maxsize defaults are fairly common and intuitive, so it would be nice to be able to use them in stubs as in your above message. It would be good though to get user feedback (@gramster?) on what kind of more complex defaults would be usable and useful for IDEs.

[AlexWaygood]

In general I'm wary of allowing defaults that can't be easily verified by stubtest. I'd want most things that can't be easily verified by stubtest to be disallowed by flake8-pyi. But I'd be happy to special-case a small number of ast.Attribute nodes in flake8-pyi, and sys.maxsize seems like an obvious one to allow.

[AlexWaygood]

The default value here is set to mmap.PAGESIZE at runtime, which is 4096 on my machine, but evidently a much larger value on Jelle's machine:

typeshed/stdlib/multiprocessing/heap.pyi

Line 30 in 7e40d70

def __init__(self, size: int = 16384) -> None: ...

[AlexWaygood]

Here's a few funny ones I found using stubtest with mypy master 😄:

typeshed/stdlib/platform.pyi

Line 41 in 7e40d70

executable: str = "/Users/jelle/py/venvs/py311/bin/python", bits: str = "", linkage: str = ""

typeshed/stdlib/pydoc.pyi

Line 61 in 7e40d70

def getdocloc(self, object: object, basedir: str = "/Users/jelle/.pyenv/versions/3.11.1/lib/python3.11") -> str | None: ...

The first one is sys.executable, which seems like it would also make sense to be special-cased.

The second is dynamically determined using a function call at runtime: https://github.com/python/cpython/blob/3847a6c64b96bb2cb93be394a590d4df2c35e876/Lib/pydoc.py#L495

[Avasam]

I just found a nice side-effect of default parameter values:

def foo(bar=None):...
# Is now equivalent to
def foo(bar: Incomplete | None = None): ...

But the former keeps Unknown | None in pyright and might be preferable (when the default value is infact None obviously, I don't think it'd replace all cases).

[Avasam]

Is there anything left to do or discuss here? Seems like we're been adding defaults to parameters for a while.

[JelleZijlstra]

Alex's TODO list above says stubgen needs updating, but that's not something we can fix in this repo.

[Avasam]

Alex's TODO list above says stubgen needs updating, but that's not something we can fix in this repo.

True, but as far as typeshed's concerned, I think #10127 does the same until then 😃