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

Recommend that YAML keys be sorted by "spec order". #320

Conversation

gouttegd
Copy link
Contributor

This PR adds a recommendation that

  1. the keys in the YAML metadata block (in both "embedded" and “external” modes) SHOULD be sorted in the same order as the corresponding slots are listed in the definition of the MappingSet class in the SSSOM model;

  2. the keys in the curie_map dictionary (i.e. the prefix names) SHOULD be sorted by alphabetical order.

This is an alternative PR to #319 (which proposed to sort the YAML metadata block by alphabetical order). “Spec order“ is not the current behaviour of sssom-py, but is preferred at least by @matentzn.

Of note, since curie_map is currently not part of the model (and therefore not listed in the definition of MappingSet), the position of the curie_map key would be unspecified under the proposed recommendation. For what it’s worth, SSSOM-Java puts it at the top of the metadata block, before any other slot.

  • docs/ have been added/updated if necessary
  • make test has been run locally (no, irrelevant)
  • tests have been added/updated (no, irrelevant)
  • CHANGELOG.md has been updated.

This commit adds a recommendation that

1) the keys in the YAML metadata block (in both "embedded" and
   "external" modes) SHOULD be sorted in the same order as the
    corresponding slots are listed in the definition of the MappingSet
    class in the SSSOM model;

2) the keys in the 'curie_map' dictionary (i.e. the prefix names) SHOULD
   be sorted by alphabetical order.
@gouttegd gouttegd self-assigned this Sep 18, 2023
Copy link
Collaborator

@matentzn matentzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with this! Thank you!

@matentzn
Copy link
Collaborator

This can be merged on Friday 22nd by anyone who sees this without permission! I just leave it Open in case there are objections that I don't know about.

@gouttegd gouttegd merged commit 28f0fdd into mapping-commons:master Sep 23, 2023
3 checks passed
@gouttegd gouttegd deleted the recommend-sorting-yaml-metadata-in-spec-order branch September 23, 2023 10:51
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.

None yet

2 participants