Skip to content
This repository has been archived by the owner on Jan 3, 2018. It is now read-only.

Python tutor links + consider removing some material #60

Merged
merged 5 commits into from
Oct 11, 2013
Merged

Python tutor links + consider removing some material #60

merged 5 commits into from
Oct 11, 2013

Conversation

juliangarcia
Copy link
Contributor

I added links to the python online tutor in order to have a more dynamic picture of the stacks in the functions notebook.

This is part of an exercise for the SWC training, round 6.

"but when Python tries to get the value of `middle`,\n",
"it discovers that it doesn't exist any longer\n",
"and reports an error.\n",
"(The error appears first because the Python interpreter gives higher priority to error messages than \"normal\" output.)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this true? I thought this sort of thing occurred because stdout is buffered and stderr is not?

@ahmadia
Copy link
Contributor

ahmadia commented Oct 10, 2013

There's another bug in NB 1,

  • "The first thing fahr_to_celsius does is called fahr_to_kelvin" -> "The first thing fahr_to_celsius does is call fahr_to_kelvin"

@ahmadia
Copy link
Contributor

ahmadia commented Oct 10, 2013

In NB 2:

  • "If we inver the loops" -> "If we invert the loops"

@ahmadia
Copy link
Contributor

ahmadia commented Oct 10, 2013

@juliangarcia - Thanks for the PR. This looks good in general, and I think the only bugs I've found were there before you tackled this. Do you mind adding a little more information to the top of the pull request summarizing your changes and why you're making them?

This is an aesthetic thing, but you should try to follow the rules for formatting Git commit messages if you'd like them to show up properly.

Also, could you make the Python Tutor link more explicit? Instead of just using [here] in your link, perhaps [at the Online Python Tutor] or similar?

@ahmadia
Copy link
Contributor

ahmadia commented Oct 10, 2013

Also, lets modify the temp variable so that we also know what unit it is in. temp_c, temp_k, or temp_f would be appropriate, because C, F, and K are all well-recognized symbols in the context of temperature.

@juliangarcia
Copy link
Contributor Author

I understand that changing the temp variables to temp_c and temp_k will be dealt with in a different PR. Other than that I have addressed the suggested changes.

Thanks!

@ahmadia
Copy link
Contributor

ahmadia commented Oct 10, 2013

Looks good. I'm +1 on merging this, but I'll leave it open for another day in case anybody else has any comments or feedback.

And good job on the commit message :)

@rgaiacs
Copy link

rgaiacs commented Oct 11, 2013

Looks good to me too.

@juliangarcia Nice job.

@ahmadia
Copy link
Contributor

ahmadia commented Oct 11, 2013

Awesome job @juliangarcia. Thanks for the review @r-gaia-cs.

ahmadia added a commit that referenced this pull request Oct 11, 2013
Python tutor links + consider removing some material
@ahmadia ahmadia merged commit 07c9170 into swcarpentry:gh-pages Oct 11, 2013
@wking wking mentioned this pull request Nov 6, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants