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

Fixes for protonation of protein and binding site detection #216

Merged
merged 5 commits into from
Mar 28, 2022

Conversation

corey-taylor
Copy link
Collaborator

@corey-taylor corey-taylor commented Mar 4, 2022

Description

Small fixes for T018 pipeline issues and CI Python 3.9 issues.

TODOs

  • commands in obabel helper function are run in the wrong order and result in docking to a structure that has no hydrogens
  • issue with number of positional arguments when running binding site detection class.
  • Under Python 3.9: Check if ´interactions´ variable is dict or a dict embedded in list (Python 3.9) - make sure we work with the dict. See error message.

Status

  • Ready to go

Copy link
Collaborator

@dominiquesydow dominiquesydow left a comment

Choose a reason for hiding this comment

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

Hi @corey-taylor, thanks a lot for fixing this - I reran the CI a couple of times, seems to be URL timeouts here and there. Let's merge this and check if the website will be rendered alright (do not quite understand the Docs fail).

@dominiquesydow
Copy link
Collaborator

@corey-taylor, did the results change? If so, we need to rerun the notebook, right?

@corey-taylor
Copy link
Collaborator Author

Hi @corey-taylor, thanks a lot for fixing this - I reran the CI a couple of times, seems to be URL timeouts here and there. Let's merge this and check if the website will be rendered alright (do not quite understand the Docs fail).

Yep, agreed with this. If the site renders fine then we'll just put it down to gremlins in the machine or something.

@corey-taylor, did the results change? If so, we need to rerun the notebook, right?

The results changed for me, yes. I didn't actually run the input ligand in the notebook, I ran 3W32's co-crystallised ligand but you'd imagine they'd be quite different. I'll re-run it and commit.

@corey-taylor
Copy link
Collaborator Author

@dominiquesydow Oh wait a sec, didn't the notebook work better when you or @AndreaVolkamer ran it last time, given I use Linux?

@dominiquesydow
Copy link
Collaborator

@corey-taylor, on second thought: in the notebook, we are testing for the resulting compounds from projects 1 and 2:

project1.OptimizedLigands.output
# NBVAL_CHECK_OUTPUT
---
[<Ligand CID: 11292933>]

and

project2.OptimizedLigands.output
# NBVAL_CHECK_OUTPUT
---
[<Ligand CID: 28693>]

With your changes, we are still getting the same compounds (we know that because the CI is not failing except for the Python 3.9 setup). Does this make sense to you? To be honest, I would have expected to see changes - but hey, happy about stable results :)


With that said, running the notebook under 3.9 indeed reproduces the CI error also locally. I won't be able to look into this tonight but will try to pick this up tomorrow.

@corey-taylor
Copy link
Collaborator Author

corey-taylor commented Mar 9, 2022

@corey-taylor, on second thought: in the notebook, we are testing for the resulting compounds from projects 1 and 2:

project1.OptimizedLigands.output
# NBVAL_CHECK_OUTPUT
---
[<Ligand CID: 11292933>]

and

project2.OptimizedLigands.output
# NBVAL_CHECK_OUTPUT
---
[<Ligand CID: 28693>]

With your changes, we are still getting the same compounds (we know that because the CI is not failing except for the Python 3.9 setup). Does this make sense to you? To be honest, I would have expected to see changes - but hey, happy about stable results :)

Ordinarily I'd be quite worried about getting consistent results when changing something as major as the protonation of the entire protein. Certainly the ranking of some compounds changed for me, although this is obviously a very small library and a fairly apolar pocket compared to other kinases so maybe that's why the end result is the same.

With that said, running the notebook under 3.9 indeed reproduces the CI error also locally. I won't be able to look into this tonight but will try to pick this up tomorrow.

For your info, nothing special about my setup. I run 3.9.7 in BunsenLabs Linux (used theTeachOpenCADD yaml). I certainly don't see the AttributeErrors so I'm not sure why the Ubuntu version is being tripped.

@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

@jaimergp could you please take a look at why conda is failing for the Python 3.7 unit tests?
E.g. https://github.com/volkamerlab/teachopencadd/runs/5716919708?check_suite_focus=true

@dominiquesydow
Copy link
Collaborator

Merging this PR - T018 runs successfully under Py3.7 and 3.9 after workaround as outlined in PR description.

Remark on CI:

  • Docs CI still failing, addressed in Pin jinja2 to fix failing docs #217
  • Unit tests CI still fails for T018, I think with a connection error (in the SI section), most probably not an issue on our end:
      ValueError: Fetching results from DoGSiteScorer API failed.
      Expected key id not found in the response.
      The response message is as follows: {'message': 'Loading PDB file'}
    
  • Unit tests CI for MacOS: Full conda install failed (at least the imports seem to not exist) - probably rerun will resolve this. If not, open new issue and PR to fix this.

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.

2 participants