Skip to content

Conversation

hamzaibrahim21
Copy link
Collaborator

@hamzaibrahim21 hamzaibrahim21 commented May 16, 2023

Description

Fixed fingerprint generator function to give a vector as an output #360
convert fpg.GetCountFingerprint(mol) to fpg.GetFingerprint(mol) in smiles_to_fp function

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

@dominiquesydow
Copy link
Collaborator

@hamzaibrahim21, could you please

  • add this PR to the PR description of the dev base branch so that we can keep track? (I think at the minute the issue is linked instead)
  • add a more descriptive title* and description

*"T007:" as prefix (see the other PR titles) also helps for a quick overview of which notebooks are worked on at the moment, thank you!


Do you know why there are conflicts with dev? Because of the direct pushs regarding T007 on dev(I see you reverted some things, so do I misunderstand)

@hamzaibrahim21 hamzaibrahim21 changed the title fix fp bug T07: Fix fp generator bug May 16, 2023
@hamzaibrahim21 hamzaibrahim21 changed the title T07: Fix fp generator bug T07: Fix fingerprint generator error May 16, 2023
@hamzaibrahim21
Copy link
Collaborator Author

@dominiquesydow Thanks for the notes, Done 👍
I pushed the changes directly into dev branch, but it was requested to do it in a PR so I reverted it and pushed it again in a different branch to be able to do a PR. Sorry for the confusion.

@mbackenkoehler
Copy link
Collaborator

@dominiquesydow I will deal with the merge. It's usually straight-forward. Just some changed memory addresses in the output etc.

@mbackenkoehler mbackenkoehler merged commit 7d8aa89 into dev May 17, 2023
@mbackenkoehler mbackenkoehler deleted the 360-dev-branch-t007-does-not-execute branch May 17, 2023 08:55
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