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: raise an informative error if pip map is passed for requirements file #97

Merged
merged 8 commits into from
May 16, 2024

Conversation

jameslamb
Copy link
Member

Fixes #87.

  • fixes the remaining issues raised by mypy (without --strict)
  • adds a pre-commit check running mypy
  • updates all the other pre-commit checks to their latest versions
    • it's been a while, figured I'd do that since I'm touching this file anyway
    • via pre-commit autoupdate

Notes for Reviewers

This won't pass CI until #94 is merged.

@jameslamb jameslamb added bug Something isn't working non-breaking Introduces a non-breaking change labels May 15, 2024
Comment on lines +13 to +14
- pip:
- pandas<1.0
Copy link
Member Author

@jameslamb jameslamb May 15, 2024

Choose a reason for hiding this comment

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

Before this PR, doing something like this would result in an error that'd require understanding the internals of rapids-dependency-file-generator.

Traceback (most recent call last):
...
TypeError: sequence item 1: expected str instance, dict found

As of this PR, I think the error message rapids-dependency-file-generator will point people pretty directly to the issue.

Traceback (most recent call last):
...
ValueError: Map inputs like {'pip': ['pandas<1.0']} are not allowed for the 'requirements' file type.

@jameslamb jameslamb marked this pull request as ready for review May 15, 2024 14:58
@jameslamb
Copy link
Member Author

The extra couple commits I just pushed here were attempts to diagnose this:

An unexpected error has occurred: CalledProcessError: command: ('/usr/bin/python', '-mvirtualenv', '/home/runner/.cache/pre-commit/repooxbc06io/py_env-python3.9', '-p', 'python3.9')
return code: 1
stdout:
    RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.9'
stderr: (none)
Check the log at /home/runner/.cache/pre-commit/pre-commit.log

(build link).

I had been using the following in .pre-commit-config.yaml:

default_language_version:
    python: python3.9

Changing that to this pattern used across most of the RAPIDS repos resolved it:

default_language_version:
    python: python3

I'm going to merge this as I don't think that's a controversial choice and it'd be easy to revert. Wanted to just explain so you know I'm not trying to sneak in some other changes on a stale approval 😅

@jameslamb jameslamb merged commit 46f142c into rapidsai:main May 16, 2024
4 checks passed
@jameslamb jameslamb deleted the mypy/enforce-mypy branch May 16, 2024 15:53
GPUtester pushed a commit that referenced this pull request May 16, 2024
## [1.13.11](v1.13.10...v1.13.11) (2024-05-16)

### Bug Fixes

* raise an informative error if pip map is passed for requirements file ([#97](#97)) ([46f142c](46f142c))
@GPUtester
Copy link

🎉 This PR is included in version 1.13.11 🎉

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
## [1.13.11](rapidsai/dependency-file-generator@v1.13.10...v1.13.11) (2024-05-16)

### Bug Fixes

* raise an informative error if pip map is passed for requirements file ([#97](rapidsai/dependency-file-generator#97)) ([e640405](rapidsai/dependency-file-generator@e640405))
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.

Enforce mypy checks
3 participants