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

Update Model subclass DoesNotExist type #1364

Merged
merged 3 commits into from Feb 21, 2023
Merged

Conversation

mateenkasim
Copy link
Contributor

@mateenkasim mateenkasim commented Feb 8, 2023

I have made things!

  • Updates stub for DoesNotExist error on model subclass
  • In VSCode, Pylance considers all DoesNotExist errors to be the same error, when the reality is that each model has its own independent DoesNotExist. This is discussed in the referenced issue on Pylance's GH, where they attribute it to the stub. This PR updates the stub using their suggestion.
try:
    # some django code
except ModelA.DoesNotExist:
    # handle ModelA error
except ModelB.DoesNotExist:
    # pylance thinks this code is unreachable before this PR

Related issues

Notes

  • In all honesty, I don't truly understand stubs – I just copied the suggestion from the folks at Pylance + formatting described in the contribution guide
  • Formatting and testing went through smoothly, except for tests/typecheck/models/test_ineritance.yml/django_contrib_gis_base_model_mixin_inheritance, which errored because I don't have something called GDAL installed? Let me know if I truly caused this error, or if it's just a machine-specific thing.

@mschoettle
Copy link
Contributor

Cool! Does it make sense to make it Final at the same time? I don't think it is meant to be overridable, is it?

@mateenkasim
Copy link
Contributor Author

mateenkasim commented Feb 9, 2023

I think that makes sense – a DoesNotExist error has one clear purpose, as opposed to something like a save function which is often overridden.

I left the Final out because it's not used much in this repo, but I'm happy to make that change!

Copy link
Contributor

@mschoettle mschoettle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

Copy link
Collaborator

@intgr intgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

django-stubs/db/models/base.pyi Outdated Show resolved Hide resolved
@intgr intgr merged commit dff2c4f into typeddjango:master Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants