Skip to content

README.md: rearrange first paragraph to be more friendly #4002

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

Merged
merged 9 commits into from
Sep 26, 2017

Conversation

elazarg
Copy link
Contributor

@elazarg elazarg commented Sep 25, 2017

The discussion of different annotations need not be the first thing the reader see.

README.md Outdated
@@ -56,6 +47,17 @@ def fib(n: int) -> Iterator[int]:
Mypy is in development; some features are missing and there are bugs.
See 'Development status' below.

Annotations

Choose a reason for hiding this comment

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

Good move 🚜

@gvanrossum
Copy link
Member

Can you move the bullet about the xml reporter elsewhere? It's a distraction at this point. I'd also fold the discussion of supported Python 3 versions into one sentence.

@elazarg
Copy link
Contributor Author

elazarg commented Sep 25, 2017

I tried something slightly different - demonstration for python 2 too. I'm not sure this is a good idea, but I couldn't find the right way to phrase it.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I'm not keen on explicit annotations for local variables, since these are correctly inferred from the initializer. Rest LG.

README.md Outdated
@@ -160,7 +168,8 @@ In Windows, the script is generally installed in
C:\>\Python34\python \Python34\Scripts\mypy PROGRAM

If you are on Windows using Python 3.3 or 3.4, and would like to use XML reports
download LXML from [PyPi](https://pypi.python.org/pypi/lxml).
download LXML from [PyPi](https://pypi.python.org/pypi/lxml).
XML based reports do not work on Python 3.3 and 3.4 on Windows.
Copy link
Member

Choose a reason for hiding this comment

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

They do work, but only if you have LXML installed. The last sentence can be deleted.

@elazarg
Copy link
Contributor Author

elazarg commented Sep 25, 2017

So should I just remove the annotations? I think it's valuable to be aware of the way to annotate variables, since most people will not read the PEP. If the example will return a list we'll have both:

from typing import List 

def fib(n: int) -> List[int]:
    a, b = 0, 1
    result: List[int] = []
    while a < n:
        result.append(a)
        a, b = b, a + b
    return result

@ilevkivskyi
Copy link
Member

@elazarg

I think it's valuable to be aware of the way to annotate variables, since most people will not read the PEP.

I am not sure here, people will most likely not read read the PEP, but we should make them read the docs (at least to some extent).

@gvanrossum
Copy link
Member

gvanrossum commented Sep 25, 2017 via email

Smaller example for python2; only show the difference
README.md Outdated
@@ -168,8 +168,7 @@ In Windows, the script is generally installed in
C:\>\Python34\python \Python34\Scripts\mypy PROGRAM

If you are on Windows using Python 3.3 or 3.4, and would like to use XML reports
Copy link
Member

Choose a reason for hiding this comment

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

If #4007 gets merged before this, this sentence can be removed, as there are wheels for all Python versions on Windows now, and installing via test-requirements will "just work".

@gvanrossum gvanrossum mentioned this pull request Sep 25, 2017
5 tasks
@elazarg
Copy link
Contributor Author

elazarg commented Sep 26, 2017

Is this OK now? I tried to make it shorter so the first part will fit in a laptop screen.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Sorry, I had pending comments that I forgot to send.

README.md Outdated

For Python 2.7, the standard annotations are written as comments:
```python
def reverse(s): # type: str -> str
Copy link
Member

Choose a reason for hiding this comment

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

Please put the # type comment on a separate line (it's the more common style).

README.md Outdated
return s[::-1]
```

See [the documentation](http://mypy.readthedocs.io/en/stable/introduction.html) for more examples.
Copy link
Member

Choose a reason for hiding this comment

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

I think the docs link should come before the Python 2 example. There should be a separate deep link for the Python 2 docs. The PEP 484 link should be moved back into the "What is mypy" section above (but without mention of Python 3.5).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how to keep it fluent, while staying clear and concise. I gave it another try.

Maybe there can be some explanation about PEP-484 in a dedicated section.

README.md Outdated
For Python 2.7, the standard annotations are written as comments:
```python
def reverse(s):
# type: str -> str
Copy link
Member

Choose a reason for hiding this comment

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

This should be # type: (str) -> str (note parentheses), otherwise it is syntax error in type comment.

README.md Outdated

For Python 2.7, you can add annotations as comments (this is also
specified in [PEP 484](https://www.python.org/dev/peps/pep-0484/)).
hints ([PEP 484](https://www.python.org/dev/peps/pep-0484/)) to your Python programs, and use mypy to type check them statically.
Copy link
Member

Choose a reason for hiding this comment

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

Not super-important, but could you please make lines in your diff 79 chars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry. I will try to keep it in mind.

README.md Outdated
```python
def reverse(s):
# type: str -> str
return s[::-1]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use a slightly less trivial example? Like is_palindrome(s) that returns s == s[::-1]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! :)
I tried to find something like that and all I came up with was reverse :(

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I'll merge it now.

@gvanrossum gvanrossum merged commit 6c66117 into python:master Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants