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

Should the type of missing be exposed publicly for use in type annotations? #1971

Open
sirosen opened this issue Apr 1, 2022 · 3 comments
Open

Comments

@sirosen
Copy link
Contributor

sirosen commented Apr 1, 2022

I almost filed a PR to make this change, but then I realized that the desirability of this is non-obvious.

I have some code which manipulates a variable like so in a pre-dump hook:

class MySchema(Schema)
    raw = fields.Raw()

    @pre_dump
    def do_pre_dump(self, obj: dict[str, Any], **kwargs: Any) -> dict[str, Any]:
        try:
            x = complex_func()
        except ValueError:
            x = missing

        return {"raw": x}

When adding annotations, how do I annotate x? Assuming complex_func() returns a T, then

x: _Missing | T = complex_func()

seems correct. But _Missing is an internal detail, not part of marshmallow's exports.

It is not necessary for x to ever have a value of missing if you have another sentinel value to use (e.g. None works for many cases). I've refactored to handle this in my specific case, so there's no urgency here.

I have a branch which renames _Missing to MissingType, exposes it, and sets _Missing = MissingType as a bit of compatibility for anyone using it. But I'm not sure if I should submit the PR?

@sirosen
Copy link
Contributor Author

sirosen commented Apr 1, 2022

I realized after posting that there's a lot more going on in my code than just a Raw field to handle the missing that comes through -- multiple nested schemas -- but I'm not sure how best to formulate a simple example that demonstrates this.
I'll leave it as-is for now, but the main point is that there is code which may use missing as a non-None sentinel value.

@deckar01
Copy link
Member

deckar01 commented Apr 1, 2022

I don't see missing documented anywhere other than its existence being enumerated.

https://marshmallow.readthedocs.io/en/stable/api_reference.html#marshmallow.missing

I think you are expected to remove the key if you want to use the default value.

@sirosen
Copy link
Contributor Author

sirosen commented Apr 1, 2022

Yeah, I realized that I was trimming too much context from my example. There wasn't in my particular case any actual need to use missing.

I need to look more, but there might be other more legitimate cases to use it. Perhaps some helper function like

def generate_myfield(*, default: None | _Missing | str = missing):
    return MyField(load_default=default)

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

No branches or pull requests

2 participants