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

added flake8-type-checking #513

Merged
merged 5 commits into from
Oct 18, 2023

Conversation

markgrahamdawson
Copy link
Contributor

@markgrahamdawson markgrahamdawson commented Oct 17, 2023

Closes cylc/cylc-admin#182

Added flake8-type-checking library to project and resolved issues it identified.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@oliver-sanders
Copy link
Member

(MyPy is failing for reasons probably unrelated to this PR)

setup.cfg Outdated Show resolved Hide resolved
@oliver-sanders
Copy link
Member

FYI: MyPy update has broken the test on master

test@master: https://github.com/cylc/cylc-uiserver/actions/runs/6547017705

@@ -97,7 +98,7 @@ def is_token_authenticated(handler: 'CylcAppHandler') -> bool:
In these cases the bearer of the token is awarded full privileges.
"""
identity_provider: JPSIdentityProvider = (
handler.serverapp.identity_provider
handler.serverapp.identity_provider # type: ignore
Copy link
Member

@MetRonnie MetRonnie Oct 17, 2023

Choose a reason for hiding this comment

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

It is better to be specific about which error we are ignoring, so that other valid errors can still be flagged. The error code is displayed in square brackets at the end of the message:

Item "None" of "Optional[ServerApp]" has no attribute "identity_provider"  [union-attr]
Suggested change
handler.serverapp.identity_provider # type: ignore
handler.serverapp.identity_provider # type: ignore[union-attr]


# Suppress the following messages:
# By default the bodies of untyped functions are not checked, consider using --check-untyped-defs
disable_error_code = annotation-unchecked
Copy link
Member

Choose a reason for hiding this comment

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

(This is not strictly needed as those messages are informational and don't fail the check)

@codecov-commenter
Copy link

codecov-commenter commented Oct 17, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1433411) 75.42% compared to head (82681cc) 75.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
- Coverage   75.42%   75.38%   -0.04%     
==========================================
  Files          12       12              
  Lines        1428     1426       -2     
  Branches      236      236              
==========================================
- Hits         1077     1075       -2     
  Misses        301      301              
  Partials       50       50              
Files Coverage Δ
cylc/uiserver/handlers.py 79.59% <ø> (-0.28%) ⬇️

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

@MetRonnie MetRonnie merged commit 9e63552 into cylc:master Oct 18, 2023
6 checks passed
MetRonnie pushed a commit to MetRonnie/cylc-uiserver that referenced this pull request Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flake8-type-checking
4 participants