Skip to content

Typo fixes that can mislead the reader - #193

Closed
santosh wants to merge 2 commits into
econchick:masterfrom
santosh:master
Closed

Typo fixes that can mislead the reader#193
santosh wants to merge 2 commits into
econchick:masterfrom
santosh:master

Conversation

@santosh

@santosh santosh commented Nov 18, 2015

Copy link
Copy Markdown

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.

ALL CAPS variable are constants that we don't change
the value of.
it's day_tuple not day_list

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown

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.

Copy link
Copy Markdown
Author

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 👍

@econchick

Copy link
Copy Markdown
Owner

I'm also very disinclined to accept any PRs from @santosh after this tweet cc @merwok

@merwok

merwok commented Nov 18, 2015

Copy link
Copy Markdown
Contributor

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.

@merwok merwok closed this Nov 18, 2015
@santosh

santosh commented Nov 19, 2015

Copy link
Copy Markdown
Author

Woo! That's great. You people have been judgemental after a tweet. That's cool.

@merwok

merwok commented Nov 19, 2015

Copy link
Copy Markdown
Contributor

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.

@santosh

santosh commented Nov 19, 2015

Copy link
Copy Markdown
Author

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.

@econchick

Copy link
Copy Markdown
Owner

@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.

@santosh

santosh commented Nov 19, 2015

Copy link
Copy Markdown
Author

So let's start?

@econchick

Copy link
Copy Markdown
Owner

@santosh I'm very strapped for time. I'd happily review pull requests, and I'm sure @merwok as well. Perhaps start with the actual code first?

@santosh

santosh commented Nov 19, 2015

Copy link
Copy Markdown
Author

@econchick What about the pulls already addressing Python 3?

#182 will probably die, because it just replaced the Python 2 code. What about #157?

@econchick

Copy link
Copy Markdown
Owner

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.

@santosh

santosh commented Nov 19, 2015

Copy link
Copy Markdown
Author

I didn't understand?

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.

4 participants