Xilinx/PYNQ_Bootcamp

Feedback thread for day2/breakout_4_intermediate_python

cramsay opened this issue · 2 comments

Feedback thread for day2/breakout_4_intermediate_python

This is a great notebook. lot's of good stuff in here for advanced students - and your explanations are really well done. Only a few things to mention here:

Just a style thing, but there's no need to have multiple Markdown cells for each paragraph. Markdown will do this for you in one cell. It makes it a little easier to navigate.

int and float are the wrong way round in the first example.

In the string example it might be good to use single quotes for one of the words, just to show that feature as well.

I remember you saying you'd purposely left out code so you could teach them but there's no file called text.txt so that cell errors.

I couldn't run the sklearn dataset cell as sklearn module was not found. I tried to install this with pip but it just hung for me.

There's a missing comma in the list used to do the scatter plot, although this might be intentional.

In the hangman game you don't instantiate the failed variable. Again, this might also be intentional.

Just adding in some comments here after PR #15. Again, I don't know the exact style this will be presented in, so any errors may well be intentional. If that's the case, just let us know and we'll close this...

  • There's a cell in the "if statements" section that includes elif d < a:. On the Z2 board, this throws a type error because we're comparing a string and an integer.
  • Nearly as Josh mentioned, the "text.txt" file I/O doesn't work. This is because text.txt is not a string (it should be enclosed in quotes), and not because the file doesn't exist. The following w argument should also be in quotes.
  • I'd really recommend avoiding sklearn. It might install quickly on an x86 (it'll use pre-built wheel) but it tries to build everything from source when we run it on the board and this will easily take up the majority of the session.
  • Again, building on Josh's comments: the matplotlib example without sklearn has a single "." instead of a "," in the list

I did really enjoy this notebook btw - especially the bug hunting cells near the end!