-
Notifications
You must be signed in to change notification settings - Fork 10
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
♻️ REFACTOR: use pydantic
instead of __init__
constructor
#10
Conversation
Also you can avoid having to write a massive docstring and simply use: https://autodoc-pydantic.readthedocs.io/en/stable/users/examples.html#showcase |
Here the (aiida-super) aiida@prnmarvelsrv3:~/envs/aiida-super/submit/relax$ bash submit.sh
PBE PseudoDojo relaxations
Usage: relax_controller.py [OPTIONS]
Try 'relax_controller.py --help' for help.
Error: Invalid value for '-w' / '--workchain-group': no Group found with LABEL<workchains/PBEsol/elph/Dq/k2>: No result was found But I think it's clear that we should validate that the group exists instead of generating it silently on the fly. |
de5c04c
to
a2c940d
Compare
ad07255
to
557628d
Compare
Breaking behaviour: In case no `Group` with `group_label` exists, submission controllers created the specified group upon instantiation. This means is the user simply had a typo in the label upon instatiation of the controller, a new (useless) group was created. Here we adapt this behaviour to check for the groups existence instead. This is done by converting the controller classes into `pydantic` `BaseModel`s and using a `validator`. Using `BaseModel` has a few added benefits: 1. Much cleaner constructor specification, that is inherited by sub classes. This becomes especially appreciable for more complex submission controllers. 2. Automatic validation on all constructor inputs. 3. Adds type hinting to all constructor inputs, which makes the classes much more user friendly and facilitates development. 4. `pydantic` `BaseModel`s come with built-in JSON serialization support, which makes it easier to store and recreate submission controllers from JSON files. If we want to set up reproducable HTC infrastructure, this may come in handy.
557628d
to
8cc3f3c
Compare
Design discussed with and approved by @giovannipizzi during a meeting last week. |
To hammer home this point: I was using Thank god AiiDA does make the cleanup easy. ^^ |
Fixes #9
Switch to using
pydantic
to annotate the constructor inputs and add validation for some inputs. This makes the code much cleaner and readable, e.g. for theBaseSubmissionController
:It's also easy and clear to write validation for the input arguments, e.g. for
group_label
:(Note that this validation is a change from the previous
get_or_create
approach. I can revert that, but I think it's the safer approach since silently creating a new group every time an instance of theBaseSubmissionController
is made with a non-existing group label seems worse than simply having the user pass an existing group.)The
FromGroupSubmissionController
then becomes:But with the
pydantic
approach the full signature is shown when using the constructor, it even shows the docstring added for each "field" as you hover over it:The validation is also quite clear, instead of returning a massive stack trace. E.g. for a little
DummyController
class I wrote:DummyController code
Other advantages are that it becomes very easy to share a specific controller instance, since
pydantic
models have ajson()
method and hence are easily serialisable and parsable from JSON.One down side is that not all field types are supported, e.g. you cannot simply pass a
Code
type to the annotation. Custom fields can be implemented if this is necessary, but I think we can go pretty far with the supported types inpydantic
.EDIT: the latter is not an issue, apparently:
https://docs.pydantic.dev/usage/types/#arbitrary-types-allowed