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

Raise error to test what cannot be stored in json config #2688

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Jan 21, 2025

No description provided.

@maxnoe
Copy link
Member Author

maxnoe commented Jan 22, 2025

With the changes made here, we are now down to the quantity, I can test if storing the string works now.

Concerning the enums, it might be better to not rely on the implicit IntEnum or StrEnum but add an explicit case that stores <Enum>.name into the json file.

We should also add a test that it's actually possible to round-trip with the config stored in the provenance, we don't have that as far as I can tell.

@kosack
Copy link
Contributor

kosack commented Jan 24, 2025

can test if storing the string works now.

What is allowed if these are specified on the command-line or in a YAML config? I guess it's just the string? There would in any case only be two options: a single string "option": "12 TeV" or separating it into value and unit, e.g. "option":{value="12",unit="TeV"}

@kosack kosack added bug module:provenance issues related to ctapipe.core.provenance labels Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug module:provenance issues related to ctapipe.core.provenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants