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

Opportunity for improvement of the type annotations of pipx.util.dedup_ordered #1270

Closed
sam-xif opened this issue Feb 27, 2024 · 0 comments · Fixed by #1271
Closed

Opportunity for improvement of the type annotations of pipx.util.dedup_ordered #1270

sam-xif opened this issue Feb 27, 2024 · 0 comments · Fixed by #1271

Comments

@sam-xif
Copy link
Contributor

sam-xif commented Feb 27, 2024

Hello pipx team,

I am working as part of a research team developing a code analysis tool for Python. One of the issues the tool discovered in pipx's codebase is an opportunity to make the type annotations for pipx.util.dedup_ordered more specific.

Here is the definition of dedup_ordered:

def dedup_ordered(input_list: List[Any]) -> List[Any]:
    output_list = []
    seen = set()
    for x in input_list:
        if x[0] not in seen:
            output_list.append(x)
            seen.add(x[0])

    return output_list

In the function body, it is clear that a List with an inner type that has more structure is expected. Based on the usage of the input_list argument in this function, and the call that is being made to it in analyze_pip_output, the parameter annotation can be changed with certainty to List[Tuple[str, Any]]. Since it is also obvious that the return type of this function matches the parameter type, that annotation can be changed as well. I have verified that making this change creates no additional type errors according to mypy:

# modification to repo is stashed
/.../pipx (main) » mypy ./src/pipx                                                                                                                                                                                                                                                                                                                                                                                                         
src/pipx/standalone_python.py:90: error: Argument 1 to "move" has incompatible type "Path"; expected "str"  [arg-type]
src/pipx/standalone_python.py:172: error: "dict" is not subscriptable, use "typing.Dict" instead  [misc]
src/pipx/commands/interpreter.py:15: error: "list" is not subscriptable, use "typing.List" instead  [misc]
src/pipx/commands/interpreter.py:37: error: "list" is not subscriptable, use "typing.List" instead  [misc]
Found 4 errors in 2 files (checked 30 source files)

# pop off stash
/.../pipx (main) » gstp                                                                                                                                                                                                                                                                                                                                                                                                                   On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   src/pipx/util.py

no changes added to commit (use "git add" and/or "git commit -a")
Dropped refs/stash@{0} (bcdde020658610723f83ae73efae94af71af78f0)

# run mypy again
/.../pipx (main*) » mypy ./src/pipx                                                                                                                                                                                                                                                                                                                                                                                                           
src/pipx/standalone_python.py:90: error: Argument 1 to "move" has incompatible type "Path"; expected "str"  [arg-type]
src/pipx/standalone_python.py:172: error: "dict" is not subscriptable, use "typing.Dict" instead  [misc]
src/pipx/commands/interpreter.py:15: error: "list" is not subscriptable, use "typing.List" instead  [misc]
src/pipx/commands/interpreter.py:37: error: "list" is not subscriptable, use "typing.List" instead  [misc]
Found 4 errors in 2 files (checked 30 source files)

Here is the diff:

-def dedup_ordered(input_list: List[Any]) -> List[Any]:
+def dedup_ordered(input_list: List[Tuple[str, Any]]) -> List[Tuple[str, Any]]:
     output_list = []
     seen = set()
     for x in input_list:

I am happy to submit a pull request, and I will do so momentarily. I opened this issue for tracking purposes as specified in the contributing guidelines.

If you are interested in learning more about the tool and how it found this issue, let me know down in the comments, or you can contact me at xifaras.s@northeastern.edu. If you find that this suggestion is not legitimate because my reasoning is incorrect or it is not worth fixing for any reason, I would be interested in understanding why.

Thank you for your consideration!

@chrysle chrysle linked a pull request Mar 1, 2024 that will close this issue
chrysle pushed a commit that referenced this issue Mar 1, 2024
* Make dedup_ordered type annotations more specific

* Remove changelog file since this patch doesn't affect end users
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 a pull request may close this issue.

1 participant