Add generate pip dependency's from conda to pre-commit

[erfannariman]

closes: #36529

First try, not sure yet how to activate local virtual env, error message right now is:

black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
flake8-pyx...........................................(no files to check)Skipped
flake8-pxd...........................................(no files to check)Skipped
isort................................................(no files to check)Skipped
Generate pip dependency from conda.......................................Failed
- hook id: pip_to_conda
- exit code: 1

Traceback (most recent call last):
  File "scripts/generate_pip_deps_from_conda.py", line 20, in <module>
    import yaml
ModuleNotFoundError: No module named 'yaml'

[WillAyd]

Hmm not sure I think this is worth adding to pre-commit - this is something that rarely gets touched

[clbarnes]

I think that, if we're relying on pre-commit to manage checks on CI, we should also be checking that development environments (i.e. those specified by environment.yml and requirements.txt) are doing the same checks when e.g. IDEs do linting. Making sure they all describe the same toolchain is important for this and should at least be part of CI. The time added to pre-commit (compared to formatting and linting) would surely be trivial?

[TomAugspurger]

I think this is exactly what should go in pre-commit. Since I don't change the deps often I never remember the check and am annoyed when CI fails :)

[MarcoGorelli]

I think you'll need additional-dependencies to add yaml as a dependency. Or you can specify system as language, I think, will take a look tomorrow

[erfannariman]

I tried using system, but it gives a weird error as if the file name is passed as argument:

black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
flake8-pyx...........................................(no files to check)Skipped
flake8-pxd...........................................(no files to check)Skipped
isort................................................(no files to check)Skipped
pyupgrade............................................(no files to check)Skipped
Generate pip dependency from conda.......................................Failed
- hook id: pip_to_conda
- exit code: 2

usage: generate_pip_deps_from_conda.py
       [-h]
       [--compare]
       [--azure]
generate_pip_deps_from_conda.py: error: unrecognized arguments: .pre-commit-config.yaml

[jreback]

yeah this is prob fine. @erfannariman you can change out of draft mode; ping on green.

[jbrockmendel]

failed with an ImportError of yaml

[erfannariman]

failed with an ImportError of yaml

Yes, same error as my initial comment. Not sure how to use local environment. Tried to go through docs of pre-commit and some issues created for the same problem, could not figure out.

https://pre-commit.com/#repository-local-hooks

[erfannariman]

Found the solution, this should work, I had to use pass_filenames: false, and use double quote for my args

[MarcoGorelli]

Nice!

Could you just remove the

args: ["--compare"]

line, so that if you do modify environment.yml, then pre-commit will fix the requirements file for you?

[erfannariman]

Nice!

Could you just remove the

args: ["--compare"]

line, so that if you do modify environment.yml, then pre-commit will fix the requirements file for you?

Yes makes sense, done.

[clbarnes]

Could this be extended to check that pre-commit uses the same tool versions as environment.yml? I know that's not true currently with flake8 but it could save some pain for contributors in the future, as pre-commit uses a different environment to the rest.

[erfannariman]

@MarcoGorelli we can have files: ^environment.yml$ so it will only run if environment.yml has changed. Would that be desirable?

[erfannariman]

Could this be extended to check that pre-commit uses the same tool versions as environment.yml? I know that's not true currently with flake8 but it could save some pain for contributors in the future, as pre-commit uses a different environment to the rest.

Not sure if I understand you correctly, but isn't the usecase for this hook that it checks your local environment?

[MarcoGorelli]

Could this be extended to check that pre-commit uses the same tool versions as environment.yml? I know that's not true currently with flake8 but it could save some pain for contributors in the future, as pre-commit uses a different environment to the rest.

I think that, once the version of flake8 has been aligned between environment.yml and pre-commit-config.yaml, then this could be added as a separate pre-commit hook (in a separate PR)

[MarcoGorelli]

@MarcoGorelli we can have files: ^environment.yml$ so it will only run if environment.yml has changed. Would that be desirable?

sounds like a good idea

[erfannariman]

ready for review @MarcoGorelli

[MarcoGorelli]

ready for review @MarcoGorelli

can you resolve the conflict please? Then I'll take another look

[clbarnes]

Could also include requirements-dev.txt, in case it was changed accidentally and no longer matches?

[erfannariman]

@MarcoGorelli yes, done

@clbarnes added requirements-dev.txt as well.

[MarcoGorelli]

Nice! I just checked this out locally and it works as intended

[jreback]

thanks @erfannariman

[jbrockmendel]

I'm seeing /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python: No module named scripts when the hook runs. any chance of changing python to python3?

[MarcoGorelli]

I'm seeing /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python: No module named scripts when the hook runs. any chance of changing python to python3?

I thought python3 was the default if you just put python, can you try upgrading pre-commit (pip install -U pre-commit) and then doing pre-commit clean?

[jbrockmendel]

Looks like on mac python will point at python3 sometime later this year when 10.16 is released. ill just be patient until then. thanks for taking a look

[MarcoGorelli]

Sorry, my bad, this uses language: system, so it does depend on what your python points to

@jbrockmendel if you change this hook to

-   repo: local
    hooks:
    -   id: pip_to_conda
        name: Generate pip dependency from conda
        description: This hook checks if the conda environment.yml and requirements-dev.txt are equal
        language: python
        entry: python -m scripts.generate_pip_deps_from_conda
        files: ^(environment.yml|requirements-dev.txt)$
        pass_filenames: false
        additional_dependencies: [pyyaml]

then does it work?

(just tried this on a laptop where python points to Python2.7 and it works for me)