-
Notifications
You must be signed in to change notification settings - Fork 392
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
Typo fixes that can mislead the reader #193
Conversation
ALL CAPS variable are constants that we don't change the value of.
it's day_tuple not day_list
@@ -42,7 +42,7 @@ In order to read a CSV/Excel file, we have to import the `csv` module from Pytho | |||
import csv | |||
``` | |||
|
|||
`MY_FILE` is defining a global - notice how it‘s all caps, a convention for variables we won't be changing. Included in this repo is a sample file to which this variable is assigned. | |||
`MY_FILE` is defining a constant - notice how it‘s all caps, a convention for variables we won't be changing the value of. Included in this repo is a sample file to which this variable is assigned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python doesn’t have real constants: as the text says, it’s a human convention to use all caps and not change them, but it’s not something enforced by the language.
I’m also -1 on the other change: Python has objects and names, not variables with values. http://nedbatchelder.com/text/names.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are called constants though in PEP-0008: https://www.python.org/dev/peps/pep-0008/#constants. Globals aren't necessarily the same thing, they can be any variable declared at the module level whereas a "constant" is identified as a full caps variable at the module level. Though Python won't treat them differently there's a semantic difference and certain expectations on them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least there is someone to support me. @daenney 👍
I agree that the tweet is in bad taste. PRs get reviews and thanks, but we’re doing this on our free time. If santosh shows more goodwill in future PRs, I will gladly review them. |
Woo! That's great. You people have been judgemental after a tweet. That's cool. |
Yes, people can have reactions after interactions in a PR or tweet. I closed the PR because the changes are IMO not improvements. As I said, I will look at other PRs from you without prejudice. |
Well, what are plans for Python 3 though? Are we going to adapt it, or not? If so, are we going to make present code compatible in both 2 and 3 (which I think will be cumbersome for the readers at beginner level)? or just support 3? There are multiple compatiblity related pulls too, I guess. PS: I am aware that twisted is not yet ported to Python 3 yet. |
@santosh I think how I'd like to approach this is have essentially two versions of each tutorial, one for py2 and one for py3. Then the reader can toggle what s/he wants to learn. |
So let's start? |
@econchick What about the pulls already addressing Python 3? #182 will probably die, because it just replaced the Python 2 code. What about #157? |
You can see that neither of those actually have separate codebase for the tutorials, it just makes the current tutorial code compatible, which as you said, can be confusing to the reader. It may be a good start to pull those PRs and work off of that. |
I didn't understand? |
Commits contain typo fixes that can mislead the reader. If you feel they are correct, merge them.
I have read this book/manual till dataviz part 2. So fixes till there only. If I read it more later, I will submit another pull request.