ESMValGroup / ESMValTool

ESMValTool: A community diagnostic and performance metrics tool for routine evaluation of Earth system models in CMIP
https://www.esmvaltool.org
Apache License 2.0
210 stars 121 forks source link

Update all pre-commit versions #3678

Open mo-gill opened 1 week ago

mo-gill commented 1 week ago

Description

Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the ๐Ÿ›  Technical or ๐Ÿงช Scientific review.


To help with the number of pull requests:

mo-gill commented 1 week ago

I have tried using both the current precommit rev (v0.3.2.9007) and the latest rev (0.4.2) but both get slightly different variations of the same error For v0.3.2.9007:

[INFO] Installing environment for https://github.com/lorenzwalthert/precommit/.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('<mo-environmentlocation>', '--vanilla', '/var/tmp/tmp0bxucmjx/script.R')
return code: 1
stdout: (none)
stderr:
    # Bootstrapping renv 0.16.0 --------------------------------------------------
    Warning: unable to access index for repository https://cran.rstudio.com/src/contrib:
      cannot open URL 'https://cran.rstudio.com/src/contrib/PACKAGES'
    Warning: unable to access index for repository https://cloud.r-project.org/src/contrib:
      cannot open URL 'https://cloud.r-project.org/src/contrib/PACKAGES'
    * Downloading renv 0.16.0 ... FAILED
    Error in bootstrap(version, libpath) : failed to download renv 0.16.0
    Calls: source ... eval.parent -> eval -> eval -> eval -> eval -> bootstrap
    Execution halted

for 0.4.2:

most recent:
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('<REDACTED>/esmvaltool-2.9.0-2/bin/Rscript', '--vanilla', '<REDACTED>/script.R')
return code: 1
stdout: (none)
stderr:
    # Bootstrapping renv 0.17.3 --------------------------------------------------
    Warning: unable to access index for repository https://cran.rstudio.com/src/contrib:
      cannot open URL 'https://cran.rstudio.com/src/contrib/PACKAGES'
    Warning: unable to access index for repository https://cloud.r-project.org/src/contrib:
      cannot open URL 'https://cloud.r-project.org/src/contrib/PACKAGES'
    * Downloading renv 0.17.3 ... FAILED
    Error in bootstrap(version, libpath) : failed to download renv 0.17.3
    Calls: source ... eval.parent -> eval -> eval -> eval -> eval -> bootstrap
    Execution halted
Check the log at /<REDACTED>pre-commit.log

I think it may be a MO specific issue related to R so have assigned to @ehogan for the time being as i will be moved onto a different project this week.

ehogan commented 1 week ago

This is a MO issue (related to our usual proxy issues) ๐Ÿ™„ I will fix this in our community environment, but in the meantime, I will suggest using this branch to all versions in the .pre-commit-config.yaml file ๐Ÿ‘

bouweandela commented 1 day ago

@ehogan Did you want to synchronize the pins here with those in environment(_osx).yml and optionally those in setup.py for a more consistent experience?

ehogan commented 23 hours ago

@ehogan Did you want to synchronize the pins here with those in environment(_osx).yml and optionally those in setup.py for a more consistent experience?

This is a tricky question ๐Ÿ˜Š One of the benefits of pre-commit (in my opinion) is that pre-commit manages its own environments for the versions defined in the pre-commit configuration file (one environment per pre-commit hook, so there are no solving issues), so if everyone installs the pre-commit hooks, everyone will be using the same version. It's true that if someone chose to run, e.g. isort on the command line (which would use the version installed in the developer's ESMValTool environment) it may behave differently to the pre-commit hook, because the pre-commit version wouldn't necessarily be aligned with the version in the ESMValTool environment, so I agree there is a benefit to aligning the versions. The disadvantages are that if we pinned all the versions in the environment(_osx).yml file, this may make the environment harder to solve, and we would need to introduce a policy on when we would update the versions (once per release?). There may be other disadvantages I haven't thought of (@valeriupredoi any thoughts?). I would be happy to pin the versions in the environment(_osx).yml file if others are happy to do so? ๐Ÿ˜Š

bouweandela commented 21 hours ago

It shouldn't be a problem to pin in environment.yml if we update regularly. The thing is that 1) not everybody uses pre-commit and 2) pre-commit doesn't quite manage its own environments, only the Python bits, for e.g. the R linters you're still dependent on whats installed in conda.

ehogan commented 19 hours ago

It shouldn't be a problem to pin in environment.yml if we update regularly. The thing is that

  1. not everybody uses pre-commit and

  2. pre-commit doesn't quite manage its own environments, only the Python bits, for e.g. the R linters you're still dependent on whats installed in conda.

Ah, true. Ok, I'll do some pinning shortly ๐Ÿ‘

ehogan commented 14 hours ago

It shouldn't be a problem to pin in environment.yml if we update regularly. The thing is that

  1. not everybody uses pre-commit and
  2. pre-commit doesn't quite manage its own environments, only the Python bits, for e.g. the R linters you're still dependent on whats installed in conda.

Ah, true. Ok, I'll do some pinning shortly ๐Ÿ‘

Now done in 16e4d6adf ๐Ÿ˜Š

ehogan commented 14 hours ago

I think I broke everything ๐Ÿ˜•