Skip to content

Update standard filename setter for external engines in OPS 2.0 #1103

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
Open
wants to merge 8 commits into
base: dev-2.0
Choose a base branch
from
Open

Update standard filename setter for external engines in OPS 2.0 #1103

wants to merge 8 commits into from

Conversation

@sroet
Copy link
Member

@sroet sroet commented Jan 27, 2022 •
edited
Loading

This does a couple things:

  • cherry-picks in Add FutureWarning for default filesetter external engines #1102
  • the default filename_setter for ExternalEngines are now RandomStringFilenames instead of FilenameSetter
  • FilenameSetter is now a subclass of RandomStringFilenames (instead of the other way around) (this was needed to still be able to use Filename setter after changing the default options)
  • FilenameSetter now returns a string that is padded up to self.length` with zeroes
  • GromacsEngine.set_filenames only resets the filename_setter if the filename_setter has a reset attribute
  • updated the tests for this behaviout
  • solved pep8 complaints on all touched files
  • dropped nose from all touched tests

part of the discussion on #1101

@sroet sroet changed the title Update standard filenamesetter for OPS 2.0 Update standard filename setter for external engines in OPS 2.0 Jan 27, 2022
@sroet sroet requested a review from dwhswenson January 27, 2022 18:55
@sroet
Copy link
Member Author

sroet commented Jan 28, 2022

failing CI is due know issue with importing openmm (or openmmtools) in notebooks is raising a warning:

Traceback:
Unexpected output fields from running code: {'stderr'}

@dwhswenson dwhswenson added the 2.0 issues and PRs for the 2.0 release label Jan 29, 2022
@sroet
Copy link
Member Author

sroet commented Feb 3, 2022

@dwhswenson all green, this is ready for review

@sroet sroet mentioned this pull request Feb 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dwhswenson dwhswenson Awaiting requested review from dwhswenson

Assignees

No one assigned

Labels

2.0 issues and PRs for the 2.0 release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sroet @dwhswenson