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

Error handling sanitize bug #85

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Conversation

frederik-sandfort1
Copy link
Collaborator

@frederik-sandfort1 frederik-sandfort1 commented Sep 10, 2024

Fixes an error that MolSanitizeErrors are caught but not treated correctly by ErrorHandling

  • fixes one line of code (replace wrong return)
  • adds a dummy test for checking
  • automatic isort on some other files

Copy link
Collaborator

@JochenSiegWork JochenSiegWork left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@@ -349,7 +349,7 @@ def transform_single(self, input_value: Any) -> Any:
elif isinstance(p_element, FilterReinserter):
iter_value = p_element.transform_single(iter_value)
except MolSanitizeException as err:
return InvalidInstance(
iter_value = InvalidInstance(
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, the filter didn't work because the pipeline just stopped for an instance (a molecule) in the middle of the pipeline when there was a MolSanitizeException . Good catch!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Jap... But not sure where this came from and why it was there at all...

Copy link
Collaborator

@c-w-feldmann c-w-feldmann left a comment

Choose a reason for hiding this comment

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

LGTM

@frederik-sandfort1 frederik-sandfort1 merged commit 8040ebe into main Sep 10, 2024
14 checks passed
JochenSiegWork pushed a commit that referenced this pull request Sep 20, 2024
* fix molsanitize exception error catching

* linting

* isort on other stuff
JochenSiegWork added a commit that referenced this pull request Sep 23, 2024
* mol2morgan_fingerprint: remove deprecated fp function

    - Remove AllChem.GetMorganFingerprintAsBitVect
      from the code because it is deprecated.
    - Add a test to ensure the bit2atom mapping works
      as intended.

* Error handling sanitize bug (#85)

* fix molsanitize exception error catching

* linting

* isort on other stuff

* Inchitomol (#86)

* fix molsanitize exception error catching

* linting

* isort on other stuff

* add inchitomol element

* Add `--check-only` flag to isort (#88)

* docu: update readme, add image, add feature calc notebook (#90)

* docu: update readme, add image, add feature calc notebook

* readme: add feature calculation example

* readme: add published Molpipeline paper link

* notebooks: add header to feature calculation

* add counted fps to test

---------

Co-authored-by: frederik-sandfort1 <[email protected]>
Co-authored-by: Christian W. Feldmann <[email protected]>
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