FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-32769: Write annotation entry for glossary by andresdelfino · Pull Request #6657 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-32769: Write annotation entry for glossary - #6657

Merged
ilevkivskyi merged 8 commits into
python:masterfrom
andresdelfino:annotations-glosary-entry
May 14, 2018
ConversationCommits (8)ChecksFiles changed
Merged

bpo-32769: Write annotation entry for glossary#6657
ilevkivskyi merged 8 commits into
python:masterfrom
andresdelfino:annotations-glosary-entry

Conversation

andresdelfino commented Apr 30, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

ilevkivskyi reviewed May 3, 2018
View reviewed changes

ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, looks good! I have just few minor comments.

Comment thread Doc/glossary.rst Outdated
An :term:`annotation` of a function or method.

For example, this function is annotated as requiring its parameters to be
int and as returning an int as well::

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Make "int" text here a reference to builtin int class here (appears twice in this sentence), I think you can use :class: for this.

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

✔️ Done

Comment thread Doc/glossary.rst Outdated
:func:`typing.get_type_hints`.
An :term:`annotation` of a global variable, or class variable.

For example, this variable is annotated as an int::

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Same comment about making "int" a reference here.

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

✔️ Done

Comment thread Doc/glossary.rst

For example, this variable is annotated as an int::

count: int = 0

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe show also an example where there is no right hand side?

class C:
    field: int

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

✔️ Done

Comment thread Doc/glossary.rst Outdated
function object.
An :term:`annotation` of a function or method.

For example, this function is annotated as requiring its parameters to be

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

"requiring" is a too strong word, I would replace with "expecting".

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

✔️ Done

Comment thread Doc/glossary.rst Outdated
An :term:`annotation` of a function or method.

For example, this function is annotated as requiring its parameters to be
int and as returning an int as well::

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

"int" -> "an int" at beginning of this line.

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Didn't use "an int" there because we are talking about two parameters, in plural. Anyway, I changed this lines a little, I hope it makes it clearer.

Comment thread Doc/glossary.rst Outdated
:func:`typing.get_type_hints`.
An :term:`annotation` of a global variable, or class variable.

For example, this variable is annotated as an int::

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I would reformulate it as this variable is annotated as taking :class:int values or similar.

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

✔️ Done

ilevkivskyi reviewed May 3, 2018
View reviewed changes
Comment thread Doc/glossary.rst Outdated

count: int = 0

When annotating variables, assignment is optional:

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Double colon here.

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

✔️ Done

ilevkivskyi reviewed May 3, 2018
View reviewed changes
Comment thread Doc/glossary.rst
attribute of a class or module object and can be accessed using
:func:`typing.get_type_hints`.
An :term:`annotation` of a global variable, or class variable.

ilevkivskyi May 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this empty line is not needed

andresdelfino May 3, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think its best to separate a concise description of the term from the example.

If you still think it's best to put them in the same paragraph, I'll make it so :)

andresdelfino commented May 11, 2018

Copy link
Copy Markdown
Contributor Author

@ilevkivskyi should I keep "class attributes" to refer to instance variables, or should I use the later to separate between data attributes (instance variables) and behavior attributes (methods)?

I think "class attributes" is somewhat ambiguous.

ilevkivskyi approved these changes May 14, 2018
View reviewed changes

ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like how this looks now. Thanks!

ilevkivskyi merged commit f2290fb into python:master May 14, 2018
andresdelfino deleted the annotations-glosary-entry branch May 14, 2018 19:06
andresdelfino added a commit to andresdelfino/cpython that referenced this pull request May 14, 2018
[3.7] bpo-32769: Write annotation entry for glossary (pythonGH-6657)
696d08f 
https://bugs.python.org/issue32769
(cherry picked from commit f2290fb)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ilevkivskyi ilevkivskyi approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL