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

allow object ids in Arg defaults #215

Merged
merged 1 commit into from
May 12, 2024
Merged

Conversation

gilesknap
Copy link
Member

This is intended to support the Vacuum Space requirement of supplying a set of args that refer to objects but that the default for all but the first is to point back to the first.

@coretl please can you verify that this is supporting what we discussed yesterday. I suggest you look at the yaml and generated startup script as follows:

@gilesknap gilesknap requested a review from coretl May 10, 2024 08:20
@coretl
Copy link
Contributor

coretl commented May 10, 2024

This makes sense, but do we need {{gauge1.name}}? Wouldn't {{gauge}} render as the same and produce the same string output, so be converted to the object in the same way?

@gilesknap
Copy link
Member Author

Well no 😀

There are two steps.The Jinja renders using the current context and always creates a string because that's what Jinja does.

At whole model validation time I then check for objects currently showing as strings and look them up by ID.

If I turn on field validation for defaults it does not work. I end up getting the string representation of the object.

Having looked at field/model validation all day today to get Collections working I may now understand why and might have a fix for this.

Will report back.

@gilesknap gilesknap merged commit 7674437 into add-collections May 12, 2024
26 checks passed
@gilesknap
Copy link
Member Author

gilesknap commented May 12, 2024

@coretl I think this is the cause of needing .name:

When the pre_init script is being rendered in Jinja it is given all of the Entities as its context with the Entity's id as the index into that context.

When an argument is being rendered it is given all its other arguments as the jinja context. Because Object Arg is not an Entity it looks like a dictionary instead and renders as {key:value ... ...}. I'm not quite sure where that conversion happens, but looking in the debugger shows ObjectArg values as dictionaries - not as Entities. The object is assigned to the ObjArg value by the Entity field validator and I guess it is doing some coercion on the return value from the validator.

@gilesknap
Copy link
Member Author

gilesknap commented May 12, 2024

Might just accept that we need .(id name) in arg defaults that ref other object args for now ...

@gilesknap
Copy link
Member Author

gilesknap commented May 12, 2024

OK I fully understand the issue. This is the offending line

arg_type = object

We create an Entity class with object arguments of type object rather than Entity. We can't make them Entity at this stage because then schema validation would happen against the base class of Entity but we want to just supply a string id and have that converted to Entity object during field validation.

@gilesknap gilesknap deleted the default-object branch May 12, 2024 18:55
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.

2 participants