feat: Add type hints

[bradenmacdonald]

This adds type hints to opaque-keys, including to the CI build so that type rules are enforced before PRs can be merged.

This is actually aimed to help with edx-platform development, because once these type hints are in place, mypy can be used to check types of modules in edx-platform - see openedx/edx-platform#32591 for example. Currently mypy just treats any OpaqueKey type as Any which is not helpful.

Some of the variables are of type ObjectId but this gets broadened automatically to Any because we're using an old version of pymongo that lacks type annotations. Once we upgrade pymongo, we'll have a more specific ObjectId type working.

Left for future:

• Upgrade pymongo so we have a working ObjectId type

[openedx-webhooks]

Thanks for the pull request, @bradenmacdonald!

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

[codecov]

Codecov Report

Patch coverage: 98.30% and project coverage change: +0.04% 🎉

Comparison is base (14d42dd) 94.07% compared to head (3675cfa) 94.12%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #256      +/-   ##
==========================================
+ Coverage   94.07%   94.12%   +0.04%     
==========================================
  Files          29       29              
  Lines        2837     2860      +23     
  Branches      292      292              
==========================================
+ Hits         2669     2692      +23     
+ Misses        143      142       -1     
- Partials       25       26       +1     

Flag	Coverage Δ
unittests	94.12% <98.30%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
opaque_keys/edx/django/tests/models.py	96.87% <ø> (ø)
opaque_keys/edx/tests/__init__.py	100.00% <ø> (ø)
opaque_keys/edx/tests/test_course_locators.py	100.00% <ø> (ø)
opaque_keys/edx/tests/test_properties.py	100.00% <ø> (ø)
setup.py	0.00% <ø> (ø)
opaque_keys/__init__.py	95.94% <94.73%> (+0.08%)	⬆️
opaque_keys/edx/locator.py	92.73% <98.83%> (+0.35%)	⬆️
opaque_keys/edx/django/models.py	86.31% <100.00%> (-1.86%)	⬇️
opaque_keys/edx/django/tests/test_models.py	100.00% <100.00%> (ø)
opaque_keys/edx/keys.py	84.25% <100.00%> (+0.92%)	⬆️
... and 4 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bradenmacdonald]

@felipemontoya @pdpinch would one of you be able to review this PR for me?

[blarghmatey]

Is it worthwhile (or even possible) to include the subtypes of the tuple returned?

[bradenmacdonald]

It would be nice, but I don't think it's even possible, as the number of fields in the tuple depends on the KEY_FIELDS of the subclass. The type isn't tuple[str, ...] either, as some of the "key field" values are actually ObjectId type, not strings.

[felipemontoya]

I'm not well versed with the type hints. I don't think I would be a good reviewer but I'm glad to see the PR is being reviewed now.

[bradenmacdonald]

@pdpinch would you be able to approve this one?

[bradenmacdonald]

Thanks for your review, @blarghmatey :)

[openedx-webhooks]

Thanks for the pull request, @bradenmacdonald! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[bradenmacdonald]

@felipemontoya or @pdpinch would one of you be comfortable approving this based on @blarghmatey's review? Sorry to ping multiple times here, but you two are the only CCs for this repo so I'm not sure how to move it forward otherwise.

[felipemontoya]

Yes. I actually learned a lot about type hints while following this PR

[felipemontoya]

Based on the review by @blarghmatey and the inclusion of mypy as part of the test suite this looks good to me.
Thanks @bradenmacdonald for the update to using type hints.

[openedx-webhooks]

@bradenmacdonald 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[bradenmacdonald]

Thanks @felipemontoya !