Skip to content
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

Additional changes for week 1 #123

Merged
merged 11 commits into from
Oct 1, 2018
Merged

Additional changes for week 1 #123

merged 11 commits into from
Oct 1, 2018

Conversation

ageorgou
Copy link
Contributor

@ageorgou ageorgou commented Sep 27, 2018

Should cover most of the comments on the "Introduction to Python" "chapter". The changes to the Classes page will go in a separate PR, since it will almost certainly be covered in Week 2.

This is mostly:

  • formatting
  • updating links, explanations
  • changing the course code
  • replacing the Google Maps API images (still to do)

Closing parenthesis was not captured as part of link, breaking it.
Fixes #60. Perhaps there is a better link for Unicode tables.
Fixes #63. Removed mention of PyPI at this stage, as it's explained
later on in the course.
Fixes #62. Now we're asking two questions, one for capacity and one
for current occupancy, and we're similarly giving two answers.
Fixes #59. Updating a link, adding some explanatory text, making
some small formatting changes.
@ageorgou
Copy link
Contributor Author

ageorgou commented Sep 28, 2018

Build seems to be failing due to an IPython version issue, investigating...

Edit: This is due to a version requirement clash for prompt-toolkit between jupyter-console and IPython. This is noted in jupyter/jupyter_console#162, and goes back to jupyter/jupyter_console#158.

More specifically, jupyter_console requires prompt-toolkit<2.0.0,>=1.0.0, whereas IPython v7 (released on PyPI recently) requires prompt_toolkit>=1.0.4,<2.0.0. Pip installes prompt-toolkit 1.0.15, which then makes IPython break during nbconvert.

@ageorgou
Copy link
Contributor Author

Requiring ipython<7 in our own requirements.txt works around this problem, so this could be a temporary fix until there is a change in the official packages.

@ageorgou
Copy link
Contributor Author

ageorgou commented Sep 28, 2018

I was trying the build on my machine and apparently set some stuff. Sorry for any confusion @jamespjh!

Of note, the latitude and longitude are switched in the new API.
Also, it returns satellite images as JPEGs instead of PNGs, which
means we can't use Matplotlib's imread.
More info on parameters etc:
https://tech.yandex.com/maps/doc/staticapi/1.x/dg/concepts/input_pa
rams-docpage/
Matplotlib's imread only handles PNG data natively. An alternative
would be to install PIL (after which imread should also be able to
handle JPEG) but I didn't try it.
@ageorgou
Copy link
Contributor Author

The only API I could find that gives static map images without requiring a key was from Yandex. Things to note:

  • The images are different, so the analysis results are also quite different (particularly some redder bits in one of the final plots)
  • The images returned are in JPEG format, so we need a different library to read them
  • It looks like we are complying with the terms of their free API, but we should double-check (especially the part about storing results): https://tech.yandex.com/maps/doc/jsapi/2.1/terms/index-docpage/

IPython 7 has a version clash with jupyter-kernel for one of their
requirements, which means that the build is failing. This is a
temporary workaround until the official packages are fixed.
@ageorgou ageorgou requested a review from dpshelio September 28, 2018 17:33
Removing reference to UCL Marketplace. Clarify requirement for
previous experience with programming. Remove requirement for own
laptops, since we'll be using the Cluster Room this year.
@dpshelio dpshelio merged commit ad992b9 into master Oct 1, 2018
@dpshelio dpshelio deleted the week1_more branch October 1, 2018 16:58
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.

2 participants