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

Remove custom action methods #1699

Merged
merged 3 commits into from Jan 22, 2024
Merged

Remove custom action methods #1699

merged 3 commits into from Jan 22, 2024

Conversation

melodyyzh
Copy link
Contributor

@melodyyzh melodyyzh commented Jan 16, 2024

Description

Removed the following methods in a branch off trunk-major after adding deprecation warnings in #1692

  • _InternalCustomUpdater.update.
  • _InternalCustomTuner.tune.
  • _InternalCustomWriter.write.
  • HDF5Log.write.

Motivation and context

Resolves #1648

How has this been tested?

Ran python tests on all the hoomd files, removed the wrap action act method in custom_operation.py to avoid the test calling the operation function and introduce error.

Change log

Removed:

* ``_InternalCustomUpdater.update``.
  (`#1699 <https://github.com/glotzerlab/hoomd-blue/pull/1699>`__).
* ``_InternalCustomTuner.tune``.
  (`#1699 <https://github.com/glotzerlab/hoomd-blue/pull/1699>`__).  
* ``_InternalCustomWriter.write``.
  (`#1699 <https://github.com/glotzerlab/hoomd-blue/pull/1699>`__).
* ``HDF5Log.write``.
  (`#1699 <https://github.com/glotzerlab/hoomd-blue/pull/1699>`__).

Checklist:

@melodyyzh melodyyzh requested review from a team as code owners January 16, 2024 23:42
@melodyyzh melodyyzh requested review from tcmoore3, tommy-waltmann and shihkual and removed request for a team January 16, 2024 23:42
@melodyyzh
Copy link
Contributor Author

The pytest failed at test_hdf5.py because it called hdf5_writer.write(), which was removed in this PR. Based on the conversation in PR #1588 , I am thinking we can either remove this line because timestep is optional, or replace it with simulation.timestep if it has been implemented in the documation?

@joaander
Copy link
Member

You can remove test_write_method from test_hdf5.py. There is no need to test a method that does not exist.

Copy link
Contributor

@tommy-waltmann tommy-waltmann left a comment

Choose a reason for hiding this comment

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

Does the changelog need to be updated to say these methods were removed?

@joaander
Copy link
Member

Does the changelog need to be updated to say these methods were removed?

Yes. I handle changelog updates separately from pull requests in HOOMD-blue and will add this at some point after the PR is merged.

What does need to be added to this PR is an update to migrating.rst with a Migrating to HOOMD-blue 5.0 section that notes the changes needed to user scripts.

@melodyyzh
Copy link
Contributor Author

pre-commit.ci autofix

@melodyyzh melodyyzh added the validate Execute long running validation tests on pull requests label Jan 17, 2024
Copy link
Contributor

@shihkual shihkual left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@tcmoore3 tcmoore3 left a comment

Choose a reason for hiding this comment

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

LGTM.

@joaander joaander changed the title Deprecate custom action methods Remove custom action methods Jan 22, 2024
@joaander joaander merged commit 77ebec8 into trunk-major Jan 22, 2024
40 checks passed
@joaander joaander deleted the remove_custom_methods branch January 22, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validate Execute long running validation tests on pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants