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

New format for validity files #10

Open
gipert opened this issue Oct 18, 2024 · 9 comments · May be fixed by #11
Open

New format for validity files #10

gipert opened this issue Oct 18, 2024 · 9 comments · May be fixed by #11

Comments

@gipert
Copy link
Member

gipert commented Oct 18, 2024

The current JSONL format produces unreadable files. I propose to switch to YAML (which, unlike JSON, fully supports lists of records):

- valid_from: 20230311T235840Z
  category: all
  reset: true
  apply:
  - l200-p03-r%-T%-all-config.json
  - l200-p03-r%-T%-cal-config.json
  - l200-p03-r000-T%-all-config.json

- valid_from: 20230317T211819Z
  category: all
  apply:
  - l200-p03-r001-T%-phy-config.json

- valid_from: 20240304T134049Z
  category: all
  reset: true
  apply:
  - l200-p42-r%-T%-all-config.json

I hope it's self-explanatory.

@oschulz
Copy link
Contributor

oschulz commented Oct 18, 2024

Makes sense - put please give both software stacks time to prepare the transition.

@gipert
Copy link
Member Author

gipert commented Oct 18, 2024

Sure. I think we should collect all these metadata changes and implement them altogether. See also https://github.com/legend-exp/legend-datasets

@gipert
Copy link
Member Author

gipert commented Oct 18, 2024

CC @ggmarshall

@ggmarshall
Copy link

ggmarshall commented Oct 21, 2024

I would propose to expand/change the reset key to also include other options.
Three main ones I see are: add to existing, remove from existing, and reset

@ggmarshall
Copy link

Change reset to mode: append, remove, reset

@ggmarshall
Copy link

@theHenks

@ggmarshall
Copy link

ggmarshall commented Nov 26, 2024

- valid_from: 20230311T235840Z
  category: all
  mode: reset
  apply:
  - l200-p03-r%-T%-all-config.json
  - l200-p03-r%-T%-cal-config.json
  - l200-p03-r000-T%-all-config.json

- valid_from: 20230317T211819Z
  category: all
  mode: append
  apply:
  - l200-p03-r001-T%-phy-config.json

- valid_from: 20240304T134049Z
  category: all
  mode: remove
  apply:
  - l200-p03-r001-T%-phy-config.json

- valid_from: 20240305T134049Z
  category: all
  mode: replace
  apply:
  - l200-p03-r000-T%-phy-config.json 
  - l200-p03-r002-T%-phy-config.json 

This is what I now have implemented, added replace mode also

@ggmarshall
Copy link

ggmarshall commented Nov 26, 2024

One additional idea would be to add a reason field also to document why a change is made, I think it is good when browsing the metadata for the user to be able to have links to docs/ slides laying out the reasoning. Obviously yaml does support comments but I think many users will just be interacting through pylegendmeta so having it as a proper field is better

@gipert
Copy link
Member Author

gipert commented Nov 26, 2024

@theHenks blessed?

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 a pull request may close this issue.

3 participants