-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: 🔧 set up mypy and fix issues from it #1403
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
Changes from all commits
27610da
ab4c5ce
a0d3028
544c1ec
72e2ec5
ddb7c83
b91880c
1282cd3
ea22e90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| [mypy] | ||
| python_version = 3.12 | ||
|
|
||
| [mypy-quartodoc.*] | ||
| ignore_missing_imports = True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,7 @@ def join_names(licenses: list[LicenseProperties] | None) -> str: | |
| Returns: | ||
| A comma-separated list of names. | ||
| """ | ||
| return ", ".join(license.name for license in licenses) if licenses else "N/A" | ||
| return ", ".join(str(license.name) for license in licenses) if licenses else "N/A" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a very common situation: we know that this property is not none (because we've checked the properties already), but mypy has no way of inferring this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have a think whether now, with the new script-based approach, it would make sense for us to use Pydantic to help with object typing? |
||
|
|
||
|
|
||
| def format_date(created: str | None) -> str: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ | |
| ) | ||
|
|
||
|
|
||
| def _read_json(path: Path) -> list | dict: | ||
| def _read_json(path: Path) -> dict: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit more correct to have |
||
| """Reads the contents of a JSON file into an object.""" | ||
| return loads(path.read_text()) | ||
|
|
||
|
|
@@ -100,7 +100,7 @@ def _validation_errors_to_check_errors( | |
| CheckError( | ||
| message=error.message, | ||
| json_path=_get_full_json_path_from_error(error), | ||
| validator=error.validator, | ||
| validator=str(error.validator), | ||
| ) | ||
| for error in _unwrap_errors(list(validation_errors)) | ||
| if error.validator not in COMPLEX_VALIDATORS | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,8 @@ def test_works_with_custom_path(tmp_path): | |
| def load_properties(path: Path) -> PackageProperties: | ||
| """Loads `properties` object from file.""" | ||
| spec = spec_from_file_location("test_module", path) | ||
| assert spec | ||
| assert spec.loader | ||
|
Comment on lines
+45
to
+46
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought this was okay to do in tests to tell mypy these are not none. If the assert fails, the test fails -- as it should. |
||
| module = module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module.properties | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.