-
-
Notifications
You must be signed in to change notification settings - Fork 232
Allow system level metadata information #3892
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
base: master
Are you sure you want to change the base?
Conversation
struct Author end | ||
struct MyVersion end | ||
struct License end | ||
struct Category end | ||
struct Tags end | ||
struct MyBool end | ||
struct NewInt end | ||
|
||
@mtkmodel TestMetadataModel begin | ||
@metadata begin | ||
Author = "Test Author" | ||
MyVersion = "1.0.0" | ||
License = "MIT" | ||
Category => "example" | ||
Tags = ["test", "demo", "metadata"] | ||
MyBool => false | ||
NewInt => 1 | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do these have to be structs? Can we have some docs around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was requested this API to avoid potential key overwrites. The setmetadata
utility also explicitly only has dispatches for ::DataType
. The intial version of the PR was a more standard Dict{Symbol, Any}
. We could have checked for existing keys when setting too imo, and reject overwrites that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in line with how symbolic metadata is stored. They don't have to be structs, just types. The reason is that this automatically avoids name conflicts since identically named types in different modules are different types, avoiding the necessity for name mangling.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Ref #3734
Add any other context about the problem here.
Are there cases where there are expression evaluations anywhere for the model parsing? Currently everything is delayed, but that means local variables are not referred to properly, and
esc
will simply convert the escaped expression, whereas we need it as parse/ expansion time.