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

Make model_bool meaningful for non-aggregated nested model fields #219

Open
lu-pl opened this issue Feb 11, 2025 · 2 comments
Open

Make model_bool meaningful for non-aggregated nested model fields #219

lu-pl opened this issue Feb 11, 2025 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@lu-pl
Copy link
Contributor

lu-pl commented Feb 11, 2025

Currently, the model_bool flag in the model config concerns only aggregated models, i.e. the flag allows to control the existential conditions for a model and whether it should show up in the aggregation or not. See issue #110 .

Backend implementers might also wish to assign None/null in case the existential condition of a non-aggregated model field is not met, e.g.:

query = """
select ?x ?a ?b[
    {
        "x": 1,
        "y": {
            "a": 2,
            "b": 3
        }
    },
    {
        "x": 1,
        "y": null
    },
    {
        "x": 1,
        "y": null
    },
]

where {
    values (?x ?a ?b) {
        (1 2 3)
        (1 2 UNDEF)
        (1 UNDEF UNDEF)
    }
}
"""

class NestedModel(BaseModel):
    config = ConfigDict(model_bool=["a", "b"])
    a: int | None
    b: int | None

class Model(BaseModel):
    x: int
    y: NestedModel | None = None

defines the existential condition for NestedModel as such, that both a and b values must be truthy for the model to be truthy and should result in

[
    {
        "x": 1,
        "y": {
            "a": 2,
            "b": 3
        }
    },
    {
        "x": 1,
        "y": null
    },
    {
        "x": 1,
        "y": null
    },
]

This is currently not possible and requires changes in how the mapper handles <BaseModel> | None annotated fields.

The current implementation of the model_bool feature causes a severe bug (issue #176) that requires deep changes in how model_bool generally works. The feature described in this issue depends on this bug fix and will be revisited after the fix is in place.

@lu-pl lu-pl added the enhancement New feature or request label Feb 11, 2025
@lu-pl lu-pl self-assigned this Feb 11, 2025
@lu-pl lu-pl added this to the v0.4.0 release milestone Feb 11, 2025
@lu-pl
Copy link
Contributor Author

lu-pl commented Feb 14, 2025

Note: A model field annotation like Model | None = None is a special case that requires a default argument for the optional type because the optional value can never come from the SPARQL response (see #218).

This should also be reflected in model sanity checking.

Related to #108 .

@lu-pl
Copy link
Contributor Author

lu-pl commented Feb 14, 2025

An option would of course be to infer None as the default value for field annotations like Model | None.

I am against that though, because this would introduce a special case for None that is only even feasible because of the Singleton-like type/instance-nature of None.

E.g. a string default value obviously cannot be asserted from Model | str, here it is plain that the default must be made explicit like Model | str = "default".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant