search

FAIRmat-NFDI / nyaml

https://fairmat-nfdi.github.io/nyaml
https://pypi.org/project/nyaml/
Apache License 2.0
1 stars 0 forks source link

Properly convert boolean values for definition attributes #12

Closed domna closed 8 months ago

domna commented 8 months ago

If currently ignoreExtraFields or ignoreExtraGroups is set the building of definitions fails, because booleans get converted to uppercase (from python conversion). This fixes it for this particular fields (but there may be problems somewhere else with boolean values, too).

domna commented 8 months ago

LGTM.

There's another issue with booleans. Namely, if you call a field "Yes" or "No" in nyaml, it is translated as a boolean and not as a string. This is related to how YAML defines booleans. I have this problem in the element field in NXelectron_level because Nobelium has the short notation "No". When converting back from nxdl to nyaml, it never puts the strings around No and then the other direction does not work anymore. Maybe we could hardcode this.

It would be nice to not interpret Yes and No as booleans at all. But I don't know whether we can tweak pyyaml to do this... it seems that the yaml specification v1.2 dropped this behaviour, but pyyaml supports only v1.1.

lukaspie commented 8 months ago

LGTM. There's another issue with booleans. Namely, if you call a field "Yes" or "No" in nyaml, it is translated as a boolean and not as a string. This is related to how YAML defines booleans. I have this problem in the element field in NXelectron_level because Nobelium has the short notation "No". When converting back from nxdl to nyaml, it never puts the strings around No and then the other direction does not work anymore. Maybe we could hardcode this.

It would be nice to not interpret Yes and No as booleans at all. But I don't know whether we can tweak pyyaml to do this... it seems that the yaml specification v1.2 dropped this behaviour, but pyyaml supports only v1.1.

Yeah, it's not ideal. From your link, there is the option to use ruamel.yaml and use its round_trip_loader function that defaults to YAML 1.2 behaviour, but is probably not ideal to mix yaml v1.1 and v1.2. I am also not sure if that package has the full capacity needed (and how well it is maintained).

lukaspie commented 8 months ago

Go ahead with merging this PR since we have the new issue anyway

domna commented 8 months ago

LGTM. There's another issue with booleans. Namely, if you call a field "Yes" or "No" in nyaml, it is translated as a boolean and not as a string. This is related to how YAML defines booleans. I have this problem in the element field in NXelectron_level because Nobelium has the short notation "No". When converting back from nxdl to nyaml, it never puts the strings around No and then the other direction does not work anymore. Maybe we could hardcode this.

It would be nice to not interpret Yes and No as booleans at all. But I don't know whether we can tweak pyyaml to do this... it seems that the yaml specification v1.2 dropped this behaviour, but pyyaml supports only v1.1.

Yeah, it's not ideal. From your link, there is the option to use ruamel.yaml and use its round_trip_loader function that defaults to YAML 1.2 behaviour, but is probably not ideal to mix yaml v1.1 and v1.2. I am also not sure if that package has the full capacity needed (and how well it is maintained).

Yes, I'm looking into it a bit and also created #15 for this. I will go ahead and merge this but we should look into the issue again, but I think it needs some testing and debugging time.