Skip to content

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

@sam-xif

Description

@sam-xif

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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions