CLI: add the aiida-pseudo family cutoffs set command
#55
Conversation
sphuber
added
the
pr/blocked
|
Was wondering if we might need to emit a warning if a stringency is specified that already exists, with the |
|
At some point will probably also need a |
sphuber
force-pushed
the
fix/050/cli-set-recommended-cutoffs
branch
2 times, most recently
from
6ece23c to
eb7c884
Compare
I'd think someone that goes through the effort of preparing the |
There was a problem hiding this comment.
Thanks @sphuber! Just added a small fix for the command docstring and a question.
One use case I'm still considering is when someone wants to change the cutoffs only for a specific element. This is currently not possible because all the elements need to provided to the set_cutoffs method. However, I can imagine I'm working on a specific class of materials and just want to increase the cutoff for oxygen for example. Would it make sense to allow this in case the stringency specified is already defined, and then just load the cutoffs and override those provided in the JSON by the user?
aiida_pseudo/cli/family.py
Outdated
| @options.STRINGENCY(required=True) | ||
| @decorators.with_dbenv() | ||
| def cmd_family_cutoffs_set(family, cutoffs, stringency): | ||
| r"""Set the recommended cutoffs for a pseudo potential family. |
There was a problem hiding this comment.
The raw format messes up your beautiful formatting 😭
| r"""Set the recommended cutoffs for a pseudo potential family. | |
| """Set the recommended cutoffs for a pseudo potential family. |
There was a problem hiding this comment.
Weird, I could have sworn that I added this because without it the \b marker didn't seem to be respected. But indeed, removing it keeps the formatting as intended
There was a problem hiding this comment.
Ah I see why, it is pydocstyle that complains about the \b marker saying I should use r when using backslashes. Will try to add an exception
|
|
||
| @cmd_family_cutoffs.command('set') | ||
| @arguments.PSEUDO_POTENTIAL_FAMILY() | ||
| @click.argument('cutoffs', type=click.File(mode='rb')) |
There was a problem hiding this comment.
Is there a reason for making the cutoffs an argument and the stringency a required option?
There was a problem hiding this comment.
Hmm, in the current conception. I think originally I intended to allow to specify multiple stringencies at once and then let one define the default. But indeed, now that only on stringency is defined at a time, maybe it makes more sense to specify it as an argument. On the other hand, leaving it like this does make it possible in the future to make the command more flexible and also allow a file with multiple stringencies defined, where the keys are the stringency name. Then it makes more sense to make the default stringency parameter an option and use the first one defined in the file as the default such that the flag becomes optional. Think that is the best option actually thinking about it now
sphuber
force-pushed
the
fix/050/cli-set-recommended-cutoffs
branch
from
eb7c884 to
beb1ee6
Compare
This command allows to set recommended cutoffs for a given family. The cutoffs are specified in a JSON file that is given as an argument and the stringency is specified through the `-s/--stringency` option. If the stringency already exists, it is simply overwritten.
sphuber
force-pushed
the
fix/050/cli-set-recommended-cutoffs
branch
from
beb1ee6 to
c47c3b2
Compare
Fixes #50
This command allows to set recommended cutoffs for a given family. The
cutoffs are specified in a JSON file that is given as an argument and
the stringency is specified through the
-s/--stringencyoption. If thestringency already exists, it is simply overwritten.