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

fix: fix return type hint for dedupe() #90

Merged
merged 4 commits into from
May 14, 2024

Conversation

jameslamb
Copy link
Member

Contributes to #87.

rapids-dependency-file-generator handles deduplicating the sets of dependencies provided to it, including pip: lists for conda environments.

This PR modifies the logic the function does that in the following ways:

  • fixes return type hint
  • refactors it so that:
    • only a single pass is done over the inputs to split requirements in to pip vs non-pip
    • assignments are done in a way that's a bit easier for a type checker to understand

This fixes these errors found by mypy:

src/rapids_dependency_file_generator/_rapids_dependency_file_generator.py:65: error: Argument 1 to "append" of "list" has incompatible type "dict[str, list[str]]"; expected "str"  [arg-type]
src/rapids_dependency_file_generator/_rapids_dependency_file_generator.py:66: error: Incompatible return value type (got "list[str]", expected "list[str | dict[str, str]]")  [return-value]
src/rapids_dependency_file_generator/_rapids_dependency_file_generator.py:66: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
src/rapids_dependency_file_generator/_rapids_dependency_file_generator.py:66: note: Consider using "Sequence" instead, which is covariant
src/rapids_dependency_file_generator/_rapids_dependency_file_generator.py:66: note: Perhaps you need a type annotation for "deduped"? Suggestion: "list[str | dict[str, str]]"
src/rapids_dependency_file_generator/_rapids_dependency_file_generator.py:425: error: Argument "dependencies" to "make_dependency_file" has incompatible type "list[str | dict[str, str]]"; expected "list[str | dict[str, list[str]]]"  [arg-type]

How I tested this

Confirmed that this behavior is covered by existing unit tests (and added one more condition there for completeness).

Ran the following to confirm that some of the mypy issues had been resolved:

mypy \
    --ignore-missing-imports \
    --explicit-package-bases \
    ./src

@jameslamb jameslamb added bug Something isn't working non-breaking Introduces a non-breaking change labels May 13, 2024
@jameslamb jameslamb merged commit ba42fd1 into rapidsai:main May 14, 2024
4 checks passed
GPUtester pushed a commit that referenced this pull request May 14, 2024
## [1.13.6](v1.13.5...v1.13.6) (2024-05-14)

### Bug Fixes

* fix return type hint for dedupe() ([#90](#90)) ([ba42fd1](ba42fd1))
@GPUtester
Copy link

🎉 This PR is included in version 1.13.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

difyrrwrzd added a commit to difyrrwrzd/dependency-file-generator that referenced this pull request Aug 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants