Skip to content

Enable reportUnknownParameterType in basedpyright#1403

Merged
nathanjmcdougall merged 5 commits intomainfrom
copilot/enable-report-unknown-parameter-type
Mar 13, 2026
Merged

Enable reportUnknownParameterType in basedpyright#1403
nathanjmcdougall merged 5 commits intomainfrom
copilot/enable-report-unknown-parameter-type

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 13, 2026 •
edited
Loading

  • Fix Key: TypeAlias = str | re.Pattern → str | re.Pattern[str] in _io.py
  • Add type annotations to Series.__getitem__ and Series.__setitem__ in containers.py
  • Fix lcs_list_update signature: use TypeVar (list[_T]) so call sites with list[int] remain valid (list is invariant, list[object] caused type errors)
  • Fix pre-existing type issue in func.py: use raise NotImplementedError instead of assert when _union() returns None unexpectedly
  • Add [object] type arg to bare KeyValueFileManager and UsethisFileManager references
  • Enable reportUnknownParameterType in pyproject.toml (remove from disabled list, add to tests environment)
  • Run prek run -a and basedpyright — all clean
Original prompt

This section details on the original issue you should resolve

<issue_title>enable reportUnknownParameterType in basedpyright</issue_title>
<issue_description>Doesn't need to be enabled for the tests dir - can stay disabled.

[[tool.basedpyright.executionEnvironments]]
root = "tests"
reportUnknownParameterType = false
reportUnreachable = false
reportUnusedFunction = false

Many violation cases seem to be that KeyValueFileManager is a generic but we're not marking it with KeyValueFileManager[object] explicitly - doing so fixes the issue.

There's also some cases in pipeweld where we're not using annotations when we should be.

Some cases are due to Key: TypeAlias = str | re.Pattern in src\usethis\_io.py. The re.Pattern is generic in the string type - we should go with Key: TypeAlias = str | re.Pattern[AnyStr].</issue_description>

Comments on the Issue (you are @copilot in this section)

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

@nathanjmcdougall
Enable reportUnknownParameterType in basedpyright
299895a 
Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com>
Copilot AI changed the title [WIP] Enable reportUnknownParameterType in basedpyright Enable reportUnknownParameterType in basedpyright Mar 13, 2026
Copilot AI requested a review from nathanjmcdougall March 13, 2026 03:28
nathanjmcdougall
nathanjmcdougall requested changes Mar 13, 2026
View reviewed changes
src/usethis/_file/yaml/update.py Outdated


def lcs_list_update(original: list, new: list) -> None:
def lcs_list_update(original: list[Any], new: list[Any]) -> None:
Copy link
Copy Markdown
Collaborator

@nathanjmcdougall nathanjmcdougall Mar 13, 2026

Choose a reason for hiding this comment

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

@copilot prefer list[object] to list[Any]

Copy link
Copy Markdown
Contributor Author

Copilot AI Mar 13, 2026

Choose a reason for hiding this comment

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

Done in 4c6efec.

src/usethis/_pipeweld/func.py Outdated
component[idx + 1] = _union(successor_component, self.step)
union = _union(successor_component, self.step)
if len(container) == 1 and container[0] == union:
assert union is not None
Copy link
Copy Markdown
Collaborator

@nathanjmcdougall nathanjmcdougall Mar 13, 2026

Choose a reason for hiding this comment

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

@copilot instead of asserting this, please raise NotImplementedError for when it's violated

Copy link
Copy Markdown
Contributor Author

Copilot AI Mar 13, 2026

Choose a reason for hiding this comment

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

Done in 4c6efec — replaced assert union is not None with if union is None: raise NotImplementedError.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@nathanjmcdougall
Address review feedback: list[object] and NotImplementedError
4c6efec 
Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com>
Copilot AI requested a review from nathanjmcdougall March 13, 2026 03:50
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 13, 2026 •
edited
Loading

Merging this PR will not alter performance

✅ 2 untouched benchmarks

Comparing copilot/enable-report-unknown-parameter-type (41e9b5a) with main (d48df44)

Open in CodSpeed

@nathanjmcdougall nathanjmcdougall mentioned this pull request Mar 13, 2026
Closed
@nathanjmcdougall
Copy link
Copy Markdown
Collaborator

nathanjmcdougall commented Mar 13, 2026

@copilot please use the usethis-qa-static-checks skill properly

@nathanjmcdougall
Fix lcs_list_update signature: use TypeVar instead of list[object]
41e9b5a 
Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 13, 2026

@copilot please use the usethis-qa-static-checks skill properly

Done in 41e9b5a. Ran uv run prek run -a and uv run basedpyright — both pass cleanly. The list[object] signature also caused a reportArgumentType error in the tests (since list is invariant), so I switched lcs_list_update to use a TypeVar (list[_T]) instead.

@nathanjmcdougall nathanjmcdougall marked this pull request as ready for review March 13, 2026 05:14
@nathanjmcdougall nathanjmcdougall merged commit 237ccfe into main Mar 13, 2026
20 checks passed
@nathanjmcdougall nathanjmcdougall deleted the copilot/enable-report-unknown-parameter-type branch March 13, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@nathanjmcdougall nathanjmcdougall nathanjmcdougall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nathanjmcdougall nathanjmcdougall

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

enable reportUnknownParameterType in basedpyright

2 participants

@nathanjmcdougall