aiida-core
aiida-core copied to clipboard
CLI: Optional sorting for `verdi code export`
When running verdi code export
, the generated yaml file is automatically sorted. However, the resulting alphabetical ordering is not what is typically found in the code yaml files in the aiida-code-registry
and aiida-resource-registry
. This PR turns the sorting off by default, and provides a command-line option to turn on the sorting.
If we don't find any other use cases for this flag, I'd also be fine to just turn off the yaml sorting alltogether, as I don't think this is the format that we'd usually want in the yaml file?
I found the previous behavior to be a slight annoyance, while setting up some unfamiliar codes by exporting them from an existing archive and modifying the resulting yaml files accordingly.
Sry, forgot to update the tests. Will do so if this change is desired.
Even if you remove the explicit sorting, the actual order is not going to be the same as those on the registries, right? It would depend on whether the pydantic.BaseModel.fields
have any particular ordering.
Even if you remove the explicit sorting, the actual order is not going to be the same as those on the registries, right? It would depend on whether the
pydantic.BaseModel.fields
have any particular ordering.
That's a good point, which I didn't really consider. I just tested it, and it seems that when calling code.Model.model_fields
as is done in the export
function, they are returned in the same order as they are defined under class Model(BaseModel)
of AbstractCode
. This is then followed by the additional fields added by the derived classes (e.g. computer
and filepath_executable
for InstalledCode
), while unset, optional fields are excluded in verdi code export
.
When changing the ordering of the fields in the yaml
file, importing the code, and then exporting it again, the initial, modified ordering from the yaml
file is not retained, but the fields are automatically sorted, as described in the paragraph above. I think this should actually be the intended behavior, as it defines a standard ordering that derives from the actual implementation. If the sorting is different for yaml
files in the registry, that should be the responsibility of the person who uploaded it there, and not the code export
functionality of AiiDA. In any case, I don't think we can guarantee 100% the same sorting all the time, but at least with sort_keys=False
in yaml.dump
, append_text
wouldn't be shown as the first field, which is something I found quite confusing with the previous behavior.
but at least with sort_keys=False in yaml.dump, append_text wouldn't be shown as the first field, which is something I found quite confusing with the previous behavior.
And perhaps others will find it surprising if the order changes and/or is not alphabetical. In the end though, these are just configuration files that are machine readable. Just thought that this change wouldn't be that impactful especially if the order is still not necessarily going to be uniform anywhere. But I don't really care that much about it going in either, so let's go ahead and add this change. Could you just please add a test for the option. And since this is (for now) just used in verdi code export
not sure if it deserves a generic option. May as well just define it directly on the command for now
And perhaps others will find it surprising if the order changes and/or is not alphabetical.
Yeah, I guess this just boils down to personal preference in the end. But at least having the option will be nice. Eventually, also having verdi computer export
[^1] would be nice, so it might be an option there, as well. But for now it could just be specific for these two commands.
[^1]: Ofc being aware of the different implementation of a Computer
as compared to a Code
, and the need for setup
and configure
as separate steps. Might have a first look at this if I get around.
Alright, this should be ready for merge. I actually changed the option from --sort-outputs
to --unsorted
, with the default as false, so that people don't get surprised by the behavior suddenly changing. For testing via file_regression.check()
, I added a new function, as I wasn't sure how to modify the existing test_code_export
to check for the unsorted output.
As always, thanks for your help and assistance @sphuber, very much appreciated :pray: