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

Testing: Add type hints to utils.py and FilterEngine, refactoring code where needed #6604

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rdimaio
Copy link
Contributor

@rdimaio rdimaio commented Mar 26, 2024

Part of #6588.

b3227b1

  • The BaseClient constructor is refactored in a way where the static type checker can detect that host and auth_host are never None.

d5d3d67

  • Type annotations in utils.py. The methods here are used in a few places, and there are some instances where the input types are unclear, so I added type: ignore comments in places where the issue seems to be with the parent function rather than the util function. Some notes below.

oidc.py

- 1 errors with message """Argument of type "str | Unknown | Any" cannot be assigned to parameter "vallist" of type "list[str]" in function "val_to_space_sep_str"

  Type "str | Unknown | Any" cannot be assigned to type "list[str]"

    "str" is incompatible with "list[str]"""".

Candidates: line 1277

filter_engine.py

- 3 errors with message """Cannot access member "append" for type "dict[str, Any]"

  Member "append" is unknown""".

Candidates: line 284, 341, 422

4d2808d

  • The linter complains about re.match(str(cfg_forced_modules), module_name) if module_name is None.

3fee6f7

  • The changes here are due to adding type hints to the util function parse_did_filter_from_string_fe in utils.py, which is then called in FilterEngine. I added type hints all around to make it easier to understand which types were being passed in FilterEngine, as well as refactoring some stuff where needed for type hinting purposes.

bari12
bari12 previously approved these changes Apr 26, 2024
@bari12
Copy link
Member

bari12 commented Apr 26, 2024

Please rebase

@rdimaio
Copy link
Contributor Author

rdimaio commented Apr 26, 2024

Rebased in latest force-push

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.

None yet

2 participants