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

Updates for 1.4.0 release #17

Merged
merged 5 commits into from
Mar 14, 2024
Merged

Updates for 1.4.0 release #17

merged 5 commits into from
Mar 14, 2024

Conversation

SGenheden
Copy link
Contributor

No description provided.

@SGenheden SGenheden requested a review from CKannas March 12, 2024 11:16
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 65.89147% with 176 lines in your changes are missing coverage. Please review.

Project coverage is 68.73%. Comparing base (cc35138) to head (8e1644a).

Files Patch % Lines
rxnutils/routes/image.py 12.05% 123 Missing and 1 partial ⚠️
rxnutils/routes/base.py 89.77% 24 Missing and 3 partials ⚠️
rxnutils/routes/readers.py 83.33% 12 Missing and 2 partials ⚠️
rxnutils/chem/reaction.py 70.00% 2 Missing and 4 partials ⚠️
rxnutils/pipeline/base.py 25.00% 3 Missing ⚠️
rxnutils/pipeline/runner.py 33.33% 1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
- Coverage   68.79%   68.73%   -0.06%     
==========================================
  Files          21       24       +3     
  Lines        2416     2911     +495     
  Branches      427      539     +112     
==========================================
+ Hits         1662     2001     +339     
- Misses        680      828     +148     
- Partials       74       82       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md Outdated

### Features

- Adding support reading and processing routes
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Adding support reading and processing routes
- Adding support for reading and processing routes

CHANGELOG.md Outdated

### Trivial changes

- Making help for pipeline runner more simple
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Making help for pipeline runner more simple
- Making help for pipeline runner simpler

docs/routes.rst Outdated
-------

The simplest route format supported is a text file, where each reaction is written as a reaction SMILES on a line.
Routes are separated by comma
Copy link
Collaborator

Choose a reason for hiding this comment

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

We say that routes are separated by comma but below with separate them with a new line.

docs/routes.rst Outdated
Reading
-------

The simplest route format supported is a text file, where each reaction is written as a reaction SMILES on a line.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should match the information from read_reaction_lists docstring.

Suggested change
The simplest route format supported is a text file, where each reaction is written as a reaction SMILES on a line.
The simplest route format supported is a text file, where each reaction is written as a reaction SMILES in a line.

docs/routes.rst Outdated
The last line of code also make sure that the second route shares mapping with the first route.


Othe readers are available
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Othe readers are available
Other readers are available

class SynthesisRoute:
"""
This encapsulates a synthesis route or a reaction tree.
It provide convinient methods for assigning atom-mapping
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
It provide convinient methods for assigning atom-mapping
It provides convenient methods for assigning atom-mapping

It is typically initiallized by one of the readers in the
`rxnutils.routes.readers` module.

The tree depth and the forward step is automatically assigned
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The tree depth and the forward step is automatically assigned
The tree depth and the forward step are automatically assigned

) -> None:
"""
Assign atom-mapping to each reaction in the route and
ensure that is is consistent from root compound and throughout
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ensure that is is consistent from root compound and throughout
ensure that is consistent from root compound and throughout

"""
Returns linear sequences or chains extracted from the route.

Each chain is a list of a dictionary representing the molecules, only the most
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Each chain is a list of a dictionary representing the molecules, only the most
Each chain is a list of dictionaries representing the molecules, only the most


def _transform_retrosynthesis_atom_mapping(tree_dict: Dict[str, Any]) -> None:
"""
Routes output from AiZynth has atom-mapping from the template-based model,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Routes output from AiZynth has atom-mapping from the template-based model,
Routes output from AiZynth have atom-mapping from the template-based model,

@SGenheden
Copy link
Contributor Author

@CKannas - fixed your remarks

@SGenheden SGenheden requested a review from CKannas March 12, 2024 13:57
@SGenheden SGenheden merged commit 7a20456 into main Mar 14, 2024
2 checks passed
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