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

T015 - Protein-ligand docking #82

Merged
merged 7 commits into from
Feb 2, 2021
Merged

T015 - Protein-ligand docking #82

merged 7 commits into from
Feb 2, 2021

Conversation

schallerdavid
Copy link
Collaborator

@schallerdavid schallerdavid commented Dec 8, 2020

Details

  • Talktorial ID: 015
  • Title: Protein ligand docking
  • Original authors: Jaime Rodríguez-Guerra, Dominique Sydow
  • Reviewer(s): Jaime Rodríguez-Guerra
  • Date of review: Dec 2020 - ...

Content review

  • Potential labels or categories (e.g. machine learning, small molecules, online APIs): XXXXXXXX
  • One line summary: Dock small molecules into a protein.
  • 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:

Other

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@dominiquesydow dominiquesydow mentioned this pull request Dec 11, 2020
27 tasks
@dominiquesydow dominiquesydow added the new talktorial New talktorial label Dec 11, 2020
@dominiquesydow dominiquesydow changed the title T015 protein ligand docking T015 - Protein ligand docking Dec 11, 2020
@dominiquesydow dominiquesydow changed the title T015 - Protein ligand docking T015 - Protein-ligand docking Dec 11, 2020
@dominiquesydow dominiquesydow linked an issue Dec 11, 2020 that may be closed by this pull request
@schallerdavid
Copy link
Collaborator Author

Hey @jaimergp,

I would be ready for a review! Hope we can manage to get Smina running on all platforms.

@jaimergp
Copy link
Contributor

Sorry for the slow response, but the review is finally here @schallerdavid! Great work!

There's plenty of comments but fear not, it's mostly typos and a couple of reworded paragraphs. The code is perfect ;) Let me know if you need some more clarification in case I was too terse!

And thanks again, we are almost there! 🥳

I'll let you know how the conda-forge submission goes ;)

@jaimergp
Copy link
Contributor

Oh, and add some # NBVAL_CHECK_OUTPUT comments to validate the output!

@schallerdavid
Copy link
Collaborator Author

@jaimergp Thanks for the suggestions, I better start using typo checkers 😄 . This notebook would be ready to merge 🎉

@jaimergp
Copy link
Contributor

jaimergp commented Jan 28, 2021

I'll wait a bit until conda-forge/staged-recipes#13793 is merged so we can get rid of bioconda in the channels.

@jaimergp
Copy link
Contributor

jaimergp commented Feb 2, 2021

Smina is now on conda-forge!

@jaimergp jaimergp merged commit 4ce7ba6 into t011-base Feb 2, 2021
@jaimergp jaimergp deleted the das-015-revamp branch February 2, 2021 11:13
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.

Talktorial 11b: Problems connecting to OPAL web services
3 participants