-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
I am going to merge this once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A handful of nits, but nothing serious. Mostly just some advice to loop in some new tools from elsewhere in the codebase to more easily do a couple of the things you're already doing.
At a high level the only concern I have is that there's no associated tests. I know for pyiron_contrib
this is not a requirement; I'll leave it to you and @jan-janssen to decide what the standard should be for pyiron_gpl
. I also recognize from context that this is likely for the potential workshop and you might not have the time between now and then to write new tests, so even if you decide to keep this repo covered "later" is a totally fair response 😉
…n this demo pacemaker.py: use ImportAlarm(); add docstring for PaceMakerJob class; change GenericParameters to InputList; change default backend to 'pyace'; use f-strings instead of .format
import from root of pyiron_base modules Co-authored-by: Jan Janssen <jan-janssen@users.noreply.github.com>
…) instead of duck-typing
Merge with master
@yury-lysogorskiy I would suggest to merge this now, then we can create a new |
No description provided.