Skip to content
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

Fixed standard symbol use in unitsystem definitions. #514

Merged
merged 2 commits into from
Apr 5, 2024

Conversation

oke464
Copy link
Contributor

@oke464 oke464 commented Apr 4, 2024

Changed symbols used in unitsystem definitions to be consistent with the standard symbol defined in the referenced unit data schema.

@ml-evs ml-evs requested a review from rartino April 4, 2024 19:59
The standard symbol is the symbol in the respective unit data schemas.
@ml-evs ml-evs force-pushed the symbol-reference-fix branch from c233cc9 to e4698a7 Compare April 4, 2024 19:59
@ml-evs
Copy link
Member

ml-evs commented Apr 4, 2024

I think I'm a bit out of the loop on this one, I assume its correct but perhaps @rartino can confirm? (As I guess the schemas at schemas.optimade.org will also need to be regenerated?)

@ml-evs ml-evs self-requested a review April 4, 2024 20:04
@vaitkus
Copy link
Contributor

vaitkus commented Apr 4, 2024

Looks good to me as well.

@oke464, could you please also fix one minor unrelated issue in the schemas/src/defs/v1.2/unitsystems/si/si_1985.yaml file? The description field incorrectly states the release date as "in 1995" instead of "in 1985".

@rartino
Copy link
Contributor

rartino commented Apr 5, 2024

Thanks @oke464! (This wasn't caught with the schemas and sanity-checks as it is an intentional feature that a unitsystem can be defined to include a unit but with a non-standard symbol; it just wasn't the intent to use that feature here.)

I took the liberty to also add the fix @vaitkus asked for and, while we are anyway doing an update, a couple of minor typographical adjustments I noticed since the big merge.

@ml-evs This indeed affected a couple of definitions/pages, in particular for unitsystems, under schemas.optimade.org. I've now updated them.

@ml-evs ml-evs merged commit 582d1bc into Materials-Consortia:develop Apr 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants