aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
431 stars 187 forks source link

ORM: Correct field type of `InstalledCode` and `PortableCode` models #6406

Closed sphuber closed 4 months ago

sphuber commented 4 months ago

Fixes #6364

The computer and filepath_files fields of the models for the InstalledCode and PortableCode classes, respectively, only defined a single type even though their serializer would convert it to a Computer and pathlib.Path instance, respectively. This would result in pydantic emitting a warning.

The reason for omitting the type returned by the serializer was that the models are used by the DynamicEntryPointCommandGroup to dynamically generate the options for the verdi code create command group. This deduces the type of the click parameter based on the field type of the pydantic model, but since click only supports specifying a single type, and not a tuple, the first element had to be taken and a warning would be emitted. Since this would be visible at every invocation of verdi code create, which would be confusing to the user, the pydantic field type was adjusted.

This is reverted since it is factually incorrect. The warning emitted by the DynamicEntryPointCommandGroup class is removed instead as this would only be relevant for developers and this is less critical.

sphuber commented 4 months ago

I remember some discussion about this, and you noted that it is hard to avoid false positives in this case. Honestly, if that is the case I'd remove this deprecation warning since it seems to be quite disruptive.

Yes, this has been discussed multiple times across various resources. I will take another stab at trying to solve the problem as it causing a lot of confusion.