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.

Sign up for GitHub

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

Jump to bottom

Rewrite reaction methods as script interface classes #4451

Merged
merged 2 commits into from
Feb 18, 2022
Merged

Rewrite reaction methods as script interface classes #4451

merged 2 commits into from
Feb 18, 2022

Conversation

pm-blanco
Copy link
Contributor

@pm-blanco pm-blanco commented Feb 15, 2022 •
edited by jngrad
Loading

Fixes #4398

Description of changes:

  • Converted the reaction methods Cython interface to a Python interface using the ScriptInterface framework
  • Now reaction methods only take keyword arguments
  • Adapted tutorials, samples, tests and benchmarks
  • Fixed minor documentation issues

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@jngrad jngrad added this to the Espresso 4.2 milestone Feb 15, 2022
@jngrad jngrad changed the title changed the cython of the reaction methods to script interface Rewrite reaction methods as script interface classes Feb 15, 2022
jngrad
jngrad reviewed Feb 15, 2022
View reviewed changes
Copy link
Member

@jngrad jngrad 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, thanks for the hard work!

@jngrad jngrad requested a review from reinaual February 15, 2022 20:20
reinaual
reinaual previously approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@reinaual reinaual left a comment

Choose a reason for hiding this comment

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

LGTM, @jngrad you can decide if it is necessary to work on any of my comments.

samples/reaction_ensemble.py Show resolved Hide resolved
src/python/espressomd/reaction_ensemble.py Outdated Show resolved Hide resolved
testsuite/python/reaction_methods.py Outdated Show resolved Hide resolved
@jngrad jngrad force-pushed the script_interface_reaction branch from efe07cc to ab619bd Compare February 17, 2022 22:03
@jngrad jngrad requested a review from reinaual February 17, 2022 22:47
@jngrad jngrad added the automerge Merge with kodiak label Feb 18, 2022
@kodiakhq kodiakhq bot merged commit b21afcd into espressomd:python Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jngrad jngrad jngrad left review comments

@reinaual reinaual reinaual approved these changes

Assignees
No one assigned
Labels
ApiChange automerge Merge with kodiak Documentation ScriptInterface
Projects
None yet
Milestone
Espresso 4.2.0
Development

Successfully merging this pull request may close these issues.

Reaction methods should be script interface classes
3 participants
@pm-blanco @reinaual @jngrad