Skip to content

Conversation

@jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Dec 11, 2020

Details

  • Talktorial ID: T011
  • Title: Querying online API webservices
  • Original authors: Jaime Rodríguez-Guerra
  • Reviewer(s): David Schaller
  • Date of review:

Content review

  • Potential labels or categories (e.g. machine learning, small molecules, online APIs): XXXXXX
  • One line summary: XXXXXXXXXX
  • The table of contents reflects the talktorial story-line; order of #, ##, ### headers is correct
  • URLs are linked with meaningful words, instead of pasting the URL directly or linking words like here.
  • I have spell-checked the notebook
  • Images have enough resolution to be rendered with quality, without being too heavy.
  • All figures have a description
  • Markdown cell content is still in-line with code cell output (whenever results are discussed)
  • I have checked that cell outputs are not incredibly long (this applies also to DataFrames)
  • Formatting looks correctly on the Sphinx render (bold, italics, figure placing)

Code review

  • Time it took to execute (approx.):
  • Variable and function names follow snake case rules (e.g. a_variable_name vs aVariableName)
  • Spacing follows PEP8 (run Black on the code cells if needed)
  • Code line are under 99 characters each (run black -l 99)
  • Comments are useful and well placed
  • There are no unpythonic idioms like for i in range(len(list)) (see slides)
  • All 3rd party dependencies are listed at the top of the notebook
  • I have marked all code cell with output referenced in markdown cells with the label # TODO: CI
  • I have identified potential candidates for a code refactor / useful functions
  • All import ... lines are at the top (practice part) cell, ordered by standard library / 3rd party packages / our own (teachopencadd.*)
  • I have update the relative paths to absolute paths.
  • List here unfamiliar libraries you find in the imports and their intended use:

@jaimergp jaimergp added the new-talktorial New talktorial label Dec 11, 2020
@jaimergp jaimergp mentioned this pull request Dec 11, 2020
27 tasks
@jaimergp
Copy link
Contributor Author

@schallerdavid This is ready for review!

@yonghui-cc, this can help you understand the basics of web APIs!

@schallerdavid
Copy link
Collaborator

Awesome work @jaimergp , the talktorial reads very well! I only found few typos.

@schallerdavid
Copy link
Collaborator

And the description in the README.md is missing.

Co-authored-by: schallerdavid <[email protected]>
@jaimergp
Copy link
Contributor Author

Awesome @schallerdavid! Thanks for the review!

@jaimergp
Copy link
Contributor Author

I'll merge for now and will self-review again when t011-base is ready.

@jaimergp jaimergp merged commit 2ecfe72 into t011-base Jan 26, 2021
@jaimergp jaimergp deleted the jrg-011-revamp branch January 26, 2021 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-talktorial New talktorial

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants