Skip to content
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

gh-126543: Docs: change "bound type var" to "bounded" when used in the context of the 'bound' kw argument to TypeVar #126584

Merged
merged 5 commits into from
Nov 11, 2024

Conversation

fofoni
Copy link
Contributor

@fofoni fofoni commented Nov 8, 2024

I only changed instances of "bound" when they were being used as an adjective. I left other instances as they were:

  1. Of course, instances of bound-as-in-binding were not touched.

  2. The docs for the TypeVar.__bound__ attribute use the word "bound" as a noun. I thought about changing it to "boundary" or "boundary type", but gave up because there is also a TypeVar.evaluate_bound method, so "bound" as a noun is kind of set in stone. One other possibility (which I believe would be a positive change) is to change "bound" to "upper bound" here, i.e.:

        .. attribute:: __bound__
     
    -      The bound of the type variable, if any.
    +      The upper bound of the type variable, if any.
     
           .. versionchanged:: 3.12
     
              For type variables created through :ref:`type parameter syntax <type-params>`,
              the bound is evaluated only when the attribute is accessed, not when
              the type variable is created (see :ref:`lazy-evaluation`).

    (I think the "bound" inside the versionchanged block is fine, I wouldn't touch it.)

  3. The docs for ParamSpec also use "bound" as a noun, like this:

    Without ParamSpec, the simplest way to annotate this previously was to use a TypeVar with bound Callable[..., Any].

    I also though about changing it to "... a TypeVar bounded by ...", but I don't think it's necessary (though I also don't think it would be a negative change). If we end up changing "bound" to "upper bound" in the item above, then maybe the same should be done here. (But note that the expression "upper bound" does not currently appear in this document.)


📚 Documentation preview 📚: https://cpython-previews--126584.org.readthedocs.build/

@JelleZijlstra
Copy link
Member

The name "evaluate_bound" isn't set in stone; it was added during the 3.14 alphas and does not yet have any compatibility constraints. However, I think it should match the bound= argument to the TypeVar constructor and the __bound__ attribute, so I wouldn't support changing the name.

Using "upper bound" instead of "bound" in the documentation seems like a good idea.

@AlexWaygood
Copy link
Member

Using "upper bound" instead of "bound" in the documentation seems like a good idea.

Agreed!

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks for this!

Doc/library/typing.rst Outdated Show resolved Hide resolved
Doc/library/typing.rst Outdated Show resolved Hide resolved
@fofoni
Copy link
Contributor Author

fofoni commented Nov 8, 2024

There. I also changed the two instances of "bound" as noun to "upper bound", as mentioned in the OP.

I still feel that the expressions "bounded" and "upper bound" may appear disconnected. Maybe the similarity of the words is enough to imply that the expressions are talking about the same thing, but maybe it's not clear for a reader not used to typing -- I'm really not sure. (Also, maybe it's just that English is not my native language.)

Currently, the first time both expressions appear is in the following snippet. Perhaps adding just this small note would be enough?

   This syntax can also be used to create bounded and constrained type
   variables::

      class StrSequence[S: str]:  # S is a TypeVar with a `str` upper bound
-          ...
+          ...                    # We also say that S is bounded by str


      class StrOrBytesSequence[A: (str, bytes)]:  # A is a TypeVar constrained to str or bytes
          ...

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 8, 2024

Currently, the first time both expressions appear is in the following snippet. Perhaps adding just this small note would be enough?

   This syntax can also be used to create bounded and constrained type
   variables::

      class StrSequence[S: str]:  # S is a TypeVar with a `str` upper bound
-          ...
+          ...                    # We also say that S is bounded by str


      class StrOrBytesSequence[A: (str, bytes)]:  # A is a TypeVar constrained to str or bytes
          ...

That seems reasonable to me! Or maybe

   This syntax can also be used to create bounded and constrained type
   variables::

-      class StrSequence[S: str]:  # S is a TypeVar with a `str` upper bound
-          ...
+      class StrSequence[S: str]:  # S is a TypeVar with a `str` upper bound;
+          ...                     # we can say that S is "bounded by `str`"


      class StrOrBytesSequence[A: (str, bytes)]:  # A is a TypeVar constrained to str or bytes
          ...

@AlexWaygood AlexWaygood added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Nov 8, 2024
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thank you!

@fofoni
Copy link
Contributor Author

fofoni commented Nov 8, 2024

I think I'm happy with it! Do I need to do anything else?

Thank you all for the very fast iteration!

@AlexWaygood
Copy link
Member

I think I'm happy with it! Do I need to do anything else?

Not from my perspective! I'll wait a little while before merging, though, in case @JelleZijlstra or anybody else has any comments (there was quite a bit of discussion on the issue!)

Co-authored-by: Jelle Zijlstra <[email protected]>
Copy link

cpython-cla-bot bot commented Nov 10, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@fofoni
Copy link
Contributor Author

fofoni commented Nov 10, 2024

Oh wow I used GitHub's "accept suggestion" button, and now it created a commit with a different email address. Do I really need to sign the CLA again? Can I force-push changing that to my email address that had already signed the CLA?

EDIT: I guess it doesn't hurt to have the CLA signed from multiple addresses. Please tell me if having multiple addresses in the commit list is not desirable for any reason.

@AlexWaygood
Copy link
Member

Oh wow I used GitHub's "accept suggestion" button, and now it created a commit with a different email address. Do I really need to sign the CLA again? Can I force-push changing that to my email address that had already signed the CLA?

EDIT: I guess it doesn't hurt to have the CLA signed from multiple addresses. Please tell me if having multiple addresses in the commit list is not desirable for any reason.

I'm pretty sure that should be fine :-)

@JelleZijlstra JelleZijlstra merged commit 434b297 into python:main Nov 11, 2024
25 checks passed
@miss-islington-app
Copy link

Thanks @fofoni for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@JelleZijlstra
Copy link
Member

No worries! We squash-merge so it doesn't really matter.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 11, 2024
… in the context of the 'bound' kw argument to TypeVar (pythonGH-126584)

(cherry picked from commit 434b297)

Co-authored-by: Pedro Fonini <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Nov 11, 2024

GH-126657 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 11, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 11, 2024
… in the context of the 'bound' kw argument to TypeVar (pythonGH-126584)

(cherry picked from commit 434b297)

Co-authored-by: Pedro Fonini <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Nov 11, 2024

GH-126658 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Nov 11, 2024
JelleZijlstra pushed a commit that referenced this pull request Nov 11, 2024
…d in the context of the 'bound' kw argument to TypeVar (GH-126584) (#126657)


(cherry picked from commit 434b297)

Co-authored-by: Pedro Fonini <[email protected]>
JelleZijlstra pushed a commit that referenced this pull request Nov 11, 2024
…d in the context of the 'bound' kw argument to TypeVar (GH-126584) (#126658)


(cherry picked from commit 434b297)

Co-authored-by: Pedro Fonini <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants