T007/22: Switch to fingerpring generator

[hamzaibrahim21]

Description

Switching fingerprint generators in all talktorials according to #330.

TODO

• T004 --- updated by Greg in T004: Switch to use fingerprint generators #330
• T007_compound_activity_machine_learning
• T022_ligand_based_screening_neural_network
• Checked if other talktorials 1-28 need switching to fingerprint generators -> no
• Checked if DL-edition talktorials need switching to fingerprint generators -> no

Status

• Ready to go

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[dominiquesydow]

Hi @hamzaibrahim21,

Thanks a lot for taking on this task 🎉
I have two quick questions before looking at this PR in more detail:

• the PR description covers more talktorials than are actually changed (I can see changes for T004/7/22), what is meant by the description, or are some talktorials still missing?
• Greg already updated T004 here and from this description, it sounds like he also made more changes to T004, do we have two versions of T004 now or did you repeat Greg's work in this PR?

I am adding @AndreaVolkamer for visibility.

[hamzaibrahim21]

Hi @dominiquesydow

• I'm sorry that my description is not clear. I mentioned the involved talktorials with the new fingerprint generator, which are 04, 07 and 22 (forgot to delete 05).

• All talktorials are checked and the DL-edition as well, so I made an extra box for them, although they have no changes.

• I repeated Greg's work, as it'd be easier to push all changes to one branch, that is branched from dev branch (If I'm not mistaken).

[dominiquesydow]

Hi @hamzaibrahim21,

Thanks a lot for the additional context! :)

All talktorials are checked and the DL-edition as well, so I made an extra box for them, although they have no changes.

I updated the PR description accordingly, thanks!

I repeated Greg's work, as it'd be easier to push all changes to one branch, that is branched from dev branch (If I'm not mistaken).

Repeating someone else's work is I think not ideal, as they have put in work to do this (and should get the credit on GitHub in terms of commits) and it costs you time as well. Did you redo it 1-to-1? If so, we could also merge Greg's branch into this PR here, just to make sure his contribution will be visible on GitHub.

[hamzaibrahim21]

I thought that would easier for controling/merging branches later and my branch would be merged easily without problems with his branch as I mentioned it in description. sorry for such misunderstanding!!
Please let me know if I should reset talktorial 04 and thank you for taking care of this PR 🙂

[dominiquesydow]

Hi @hamzaibrahim21, no worries at all! My apologies for adding complexity to this PR but I do prefer resetting T004 (could you do that?) and merging Greg's PR into this one (I am going to do that). I think you are right that we might run into conflict issues but I will give solving them a try so that we can include Greg's commit history included. Fingers crossed :)

[hamzaibrahim21]

@dominiquesydow Great! now T04 should be reset, if anything I should do or take care of, please let me know.

[dominiquesydow]

Hi @hamzaibrahim21,
Thanks a lot! Only requested changes are nit-pits really as in adding/removing spaces in order to be in line with our formatting. Once changes, happy to go ahead with the merge if @mbackenkoehler agrees.
I did merge Greg's changes for T004 into this branch here (let me double-check in a moment if I missed any spaces there -> I checked, T004 all good & ready to go from my side).

@mbackenkoehler, I did not find another way to deal with our merge conflicts in T004 than merging changes and resolving conflicts. This is not pretty regarding the commit history but I did not know how to do this better, sorry.

[mbackenkoehler]

@hamzaibrahim21 FYI, you can just run black -l 99 talktorial.ipynb to automatically format the notebook correctly.