Skip to content

Add mypy annotations - #2283

Open
Ninjaclasher wants to merge 1 commit into
DMOJ:masterfrom
Ninjaclasher:mypy-annotate
Open

Add mypy annotations#2283
Ninjaclasher wants to merge 1 commit into
DMOJ:masterfrom
Ninjaclasher:mypy-annotate

Conversation

@Ninjaclasher

@Ninjaclasher Ninjaclasher commented Aug 27, 2023

Copy link
Copy Markdown
Member

Supercedes #1495. Refs #1493.

@Ninjaclasher
Ninjaclasher force-pushed the mypy-annotate branch 13 times, most recently from 2a3e54f to 0e2ef59 Compare August 27, 2023 22:07
@codecov-commenter

codecov-commenter commented Aug 27, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 93.35% and project coverage change: +1.12% 🎉

Comparison is base (79be4af) 46.84% compared to head (ba67723) 47.96%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2283      +/-   ##
==========================================
+ Coverage   46.84%   47.96%   +1.12%     
==========================================
  Files         249      249              
  Lines       13158    13453     +295     
==========================================
+ Hits         6164     6453     +289     
- Misses       6994     7000       +6     
Files Changed Coverage Δ
dmoj/celery.py 81.25% <0.00%> (+1.25%) ⬆️
judge/bridge/base_handler.py 0.00% <0.00%> (ø)
judge/event_poster_ws.py 0.00% <0.00%> (ø)
judge/management/commands/generate_sitemap.py 0.00% <0.00%> (ø)
judge/management/commands/runmoss.py 0.00% <0.00%> (ø)
judge/utils/pdfoid.py 45.00% <ø> (ø)
judge/utils/problem_data.py 15.02% <0.00%> (+0.40%) ⬆️
judge/widgets/select2.py 56.04% <ø> (+0.98%) ⬆️
judge/views/tasks.py 42.55% <50.00%> (+1.24%) ⬆️
judge/views/two_factor.py 40.22% <50.00%> (+0.34%) ⬆️
... and 28 more

... and 43 files with indirect coverage changes

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

@Ninjaclasher
Ninjaclasher marked this pull request as ready for review August 27, 2023 22:14
@Ninjaclasher
Ninjaclasher requested review from Xyene and quantum5 August 27, 2023 22:17
Comment thread dmoj/settings.py
from jinja2 import select_autoescape

import django_stubs_ext # noqa: I100, I102, I202
django_stubs_ext.monkeypatch()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it necessary to keep monkeypatch() out of TYPE_CHECKING?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, we need it when running in production as well. Otherwise, we still get the same TypeError (ref: https://github.com/typeddjango/django-stubs#i-cannot-use-queryset-or-manager-with-type-annotations)

Comment thread judge/management/commands/generate_sitemap.py
Comment thread judge/views/submission.py
Comment thread .github/workflows/build.yml Outdated
Comment thread dmoj/settings.py
# Set to 2 to always use HTTPS for links
# Set to 0 to always use HTTP for links
DMOJ_SSL = 0
DMOJ_SSL: int = 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we use Literal[0,1,2] instead of `int?

Comment thread dmoj/settings.py
DMOJ_TOTP_TOLERANCE_HALF_MINUTES = 1
DMOJ_SCRATCH_CODES_COUNT = 5
DMOJ_USER_MAX_ORGANIZATION_COUNT = 3
DMOJ_SUBMISSION_SOURCE_VISIBILITY: Union[Literal['all'], Literal['all-solved'], Literal['only-own']] = 'all-solved'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does Literal['all', 'all-solved', 'only-own'] not work?

def split_path_first(path, repath=re.compile('[%s]' % re.escape(os.sep + os.altsep))):
repath = re.compile('[%s]' % re.escape(os.sep + os.altsep))

def split_path_first(path):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change seems semantically different to me; are you sure we don't (and won't need to) pass repath as a argument to this function?

Comment thread judge/views/comment.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

needs an update due to #2362

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.

4 participants