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

Small fixes in T021 #153

Merged
merged 4 commits into from
Oct 14, 2021
Merged

Small fixes in T021 #153

merged 4 commits into from
Oct 14, 2021

Conversation

t-kimber
Copy link
Contributor

@t-kimber t-kimber commented Oct 13, 2021

This PR fixes small things in T021:

  • ChEMBL25 is used in the notebook. In the theory, v17 was mentioned. This has been updated and numbers are taken from here: https://ftp.ebi.ac.uk/pub/databases/chembl/ChEMBLdb/releases/chembl_25/chembl_25_release_notes.txt
  • SMILES and atoms were sometimes in quotes, sometimes in code style. Now all is in code style.
  • Quotes were not consistent (single, forward double, etc.), now all consistent to " .
  • Numbers were not in math mode.
  • Remove extra long cell output.
  • Remove unused library reference (timeit).
  • Remove the bracket when mention a function in markdown.
  • Minor formulation edits.

Note:
I still think that we should remove the suppl. material in this notebook.

Status

  • Ready to go

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@t-kimber t-kimber self-assigned this Oct 13, 2021
Copy link
Member

@AndreaVolkamer AndreaVolkamer left a comment

Choose a reason for hiding this comment

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

@t-kimber looks great. Thanks for the detailed update.
Ready to be merged!

@t-kimber
Copy link
Contributor Author

Great, thanks @AndreaVolkamer !

@dominiquesydow, can you just confirm that the tests that are failing are still okay to be merged on master or am I breaking something which is passing on master?

@dominiquesydow
Copy link
Collaborator

Great, thanks @AndreaVolkamer !

@dominiquesydow, can you just confirm that the tests that are failing are still okay to be merged on master or am I breaking something which is passing on master?

Could you please re-generate the READMEs with

for path in teachopencadd/talktorials/T*/talktorial.ipynb; do
    python devtools/regenerate_readmes.py --output README.md $path
done

The Windows fail is odd, it does not say why. I will check it again when the README pushs triggered rerunning the CI.

@dominiquesydow
Copy link
Collaborator

@t-kimber, I think you can merge this now :)

The "This check failed" problem under Windows could be temporary from GHA's side

The Docs fail due to "broken" links that are not broken...
Remindes me of this issue, which I haven't had the time to follow up on, yet:

Will open an issue to address all this separately :)

@t-kimber t-kimber merged commit 2f2dda8 into master Oct 14, 2021
@t-kimber t-kimber deleted the tk-t021-suppl branch October 14, 2021 15:42
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.

3 participants