Changes function to key in docs of wrap_numbers()

[sobolevn]

Summary

• OS: -
• Bug fix: no
• Type: doc
• Fixes: -

Description

Previously the wording was quite confusing: name is not a function (in a CPython's sense, like Callable[...]) and never used like one.
I've changed the wording to be just key, which name is. Maybe there are better ways to describe it?

Context: we are working on type stubs for psutil and found this description quite confusing python/typeshed#6104 (comment)

[giampaolo]

Mmm... interesting.

As for the bikeshedding around naming: personally I think the wording is correct, in that it explicitly states that the argument "name" refers to a function name, and not a callable, otherwise the argument would have been called "fun" or something. But then again, I wrote that, so I suppose there's some inevitable bias. Since you're closer to description from a typing perspective than I am, I'll trust your judgement and merge your PR.

Regardless, I'm interested in what your doing there. I noticed https://github.com/python/typeshed/blob/a9227ed98589bf960b1c9ee2da69890171f8dfd1/stubs/psutil/psutil/__init__.pyi. Can something like this be included in psutil itself? I'm not familiar with PEP 561, so I'm not sure.

Also, while I'm at it, I have an advice about https://github.com/python/typeshed/blob/a9227ed98589bf960b1c9ee2da69890171f8dfd1/stubs/psutil/psutil/__init__.pyi: I think you should avoid relying on platform logic to check whether something is available as in:

if sys.platform == "linux":
    from ._pslinux import (
        IOPRIO_CLASS_BE as IOPRIO_CLASS_BE,
        IOPRIO_CLASS_IDLE as IOPRIO_CLASS_IDLE,
        IOPRIO_CLASS_NONE as IOPRIO_CLASS_NONE,
        IOPRIO_CLASS_RT as IOPRIO_CLASS_RT,
    )

...because:

1. that logic already exists in psutil, so you're duplicating it
2. it may change in future versions on psutil

I would recommend using psutil.__all__ instead, as in:

from psutil import __all__

if 'IOPRIO_CLASS_BE' in __all__:
      from psutil import IOPRIO_CLASS_BE

[sobolevn]

Thanks a lot for the detailed response!

I think the wording is correct, in that it explicitly states that the argument "name" refers to a function name, and not a callable

So, function name can be a good candidate as well. I will update the PR.

Regardless, I'm interested in what your doing there. I noticed https://github.com/python/typeshed/blob/a9227ed98589bf960b1c9ee2da69890171f8dfd1/stubs/psutil/psutil/__init__.pyi. Can something like this be included in psutil itself?

Yes! We can add this to psutil directly, moreover, this is a prefered way. Storing annotations with the source code is good for several reasons:

1. It allows to use type checkers while developing psutil itself
2. It stores different annotations for different versions. Right now we only have one single typeshed for all possible versions of psutil, which is not the ideal (but only) solution

There are several ways to store annotations within the source code:

1. Apply them to the source code. It is the easiest and the most reliable way. But, it has a big problem: no python2 and <=python3.4 support. Limited support of python3.5. I think that psutil supports these versions and there's no plan to drop them
2. Store annotations as type comments like def some(a, b): # type: (a: int, b: int) -> int. This way we can still have one source of truth, but support older versions of python.
3. Store .pyi files near the source files. So, for example, you would have __init__.py and __init__.pyi with annotations only. It will store only typed signatures and imports / global constants. It is not as easy as option 1, but supports all versions of python.

What do you think?

If you want, I can send a PR with annotations I wrote for typeshed directly to this repo. Just tell me which format should we use (my personal opinion is that 2 will work the best for this case).

Also, while I'm at it, I have a complaint about https://github.com/python/typeshed/blob/a9227ed98589bf960b1c9ee2da69890171f8dfd1/stubs/psutil/psutil/__init__.pyi: I think you should avoid relyinng on platform logic to check whether something is available as in:

Unfortunatelly, we have to 😞
The point of this is to have the same global definitions you have in psutil.
And mypy can understand our if cases with sys.platform checks. So, we would have different results for type-checking on different platforms.

We cannot use something like you've proposed:

from psutil import __all__

if 'IOPRIO_CLASS_BE' in __all__:
      from psutil import IOPRIO_CLASS_BE

For several reasons:

1. psutil is not installed in our env
2. type checkers do not "execute" .pyi files the same way they do for .py files. Quick example:

example/__init__.py:

import sys

__all__ = []

if sys.platform == 'darwin':
    x = 1
    __all__.append('x')

example/__init__.pyi:

raise ValueError('Code bellow is not reachable')
x: str

mypy will report for mypy -c 'from returns import x; reveal_type(x)': <string>:1: note: Revealed type is "builtins.str"

[giampaolo]

So, function name can be a good candidate as well. I will update the PR.

Yeah, good idea. Please let's stick with fun_name (shorter).

We cannot use something like you've proposed for several reasons [...]

Oh! Understood.

There are several ways to store annotations within the source code:

1. Apply them to the source code. It is the easiest and the most reliable way. But, it has a big problem: no python2 and <=python3.4 support. Limited support of python3.5. I think that psutil supports these versions and there's no plan to drop them

Correct. We go as far back as Python 2.6!
It occurred to me there is a ticket for this already: #1946.

2. Store annotations as type comments like def some(a, b): # type: (a: int, b: int) -> int. This way we can still have one source of truth, but support older versions of python.

Can annotations be added to function docstrings instead of being appended as EOL comments?
Also, can this approach based on comments be applied to namedtuple values as well?
This looks like the best approach in terms of portability + code reuse (aka, avoid if platform == 'something' checks in .pyi` files), but at the expense of code readability.

3. Store .pyi files near the source files. So, for example, you would have init.py and init.pyi with annotations only. It will store only typed signatures and imports / global constants. It is not as easy as option 1, but supports all versions of python.

If 2) don't allow to put annotations into docstrings, then I'm keen on thinking this would probably be the best compromise.

[sobolevn]

Can annotations be added to function docstrings instead of being appended as EOL comments?

The best we can do is:

def embezzle(self, account, funds=1000000, *fake_receipts):
    # type: (str, int, *str) -> None
    """Embezzle funds from account using fake receipts."""
    <code goes here>

Source: https://www.python.org/dev/peps/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code

Also, can this approach based on comments be applied to namedtuple values as well?

It would require another hack:

from typing_extensions import NamedTuple
X = NamedTuple('X', [('a', int), ('b', str)])

However, I am not sure typing_extensions support python2.6

but at the expense of code readability.

This is very personal, because for me - type hinting is a massive reability boost.

[giampaolo]

Mmm. If we stick with option 3), do .pyi files have to live in the same dir of the .py file?
Can they be put in another, separate dir? I see there is also a METADATA.toml. Is it needed as well? Where would it go?
Let's say we do this for the main namespace only (psutil/__init__.py and psutil/_common.pyi) what the final result will look like?

However, I am not sure typing_extensions support python2.6

It doesn't matter (I will remove 2.6 eventually). Personally I wouldn't mind if typing support would be 3.X only... if that means the end result is better / cleaner / more maintainable etc. The only real constraint that we have is avoid breaking 2.7 code (that's gonna stay for many years to come).

[giampaolo]

Also, another thing to consider is testing. Right now we have unit tests that check return types of all public APIs, including namedtuple values:
https://github.com/giampaolo/psutil/blob/master/psutil/tests/test_contracts.py#L204
Can we add checks to make sure that the definitions in the .pyi files are correct? Will that require a third-party lib? (if so, it would be a new dep to add to the Makefile: https://github.com/giampaolo/psutil/blob/master/Makefile#L11).

[sobolevn]

If we stick with option 3), do .pyi files have to live in the same dir of the .py file?

We can store them separately in psutil-stubs top-level folder.

I see there is also a METADATA.toml. Is it needed as well?

Nope.

Let's say we do this for the main namespace only (psutil/init.py and psutil/_common.pyi) what the final result will look like?

It will be just three extra files: psutil/__init__.pyi, psutil/_common.pyi, and psutil/py.typed (empty file).

Can we add checks to make sure that the definitions in the .pyi files are correct?

I don't think so. At least, I don't know how to do that.

[giampaolo]

Sounds good. If you want to go for it, feel free to make a PR.

[sobolevn]

I will!