search

LupoA / lsdensities

Smeared spectral densities from lattice correlators
GNU General Public License v3.0
3 stars 0 forks source link

question about pre-commit #48

Open LupoA opened 5 months ago

LupoA commented 5 months ago

The config file in the repository reads

default_language_version:
    python: python3.11

which did not work for me, until I changed locally to my python version

default_language_version:
    python: python3.10.12

this looks a bit user-dependent. Is it a bug?

LupoA commented 5 months ago

mmmmmmmmmmmmmmmmmmm I might have addressed this in my last commit

LupoA commented 5 months ago

also: should we force pre-commit on users who intend to contribute? for instance by adding it as a dependency? or following these instructions

edbennett commented 5 months ago

I think this is the opposite of user-dependent—it's specifying a version for pre-commit to test as its default, rather than "whatever is on the system/it finds first" (I'm not actually sure what the default behaviour is in the absence of this flag).

Can you elaborate on what "did not work for me" means?

We should request pre-commit for contributors, but adding it as a dependency in pyproject.toml isn't appropriate for this. See #47. Pre-commit.ci is also good to have, and I'd recommend including that with #26.

LupoA commented 5 months ago

I think this is the opposite of user-dependent—it's specifying a version for pre-commit to test as its default, rather than "whatever is on the system/it finds first" (I'm not actually sure what the default behaviour is in the absence of this flag).

Isn't this going to fail unless the user has that very specific version of python? I've modified the config file to have python3. which I think includes all 3.x and ended up working for myself

Can you elaborate on what "did not work for me" means?

When the config file was set on python 3.11 I would get the following error

stdout:
RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.11'
stderr: (none)

We should request pre-commit for contributors, but adding it as a dependency in pyproject.toml isn't appropriate for this. See #47. Pre-commit.ci is also good to have, and I'd recommend including that with #26.

cool, thanks for explaining!

edbennett commented 5 months ago

Isn't this going to fail unless the user has that very specific version of python? I've modified the config file to have python3. which I think includes all 3.x and ended up working for myself

This works, but makes the line relatively pointless—nobody is using Python 2 these days.

When the config file was set on python 3.11 I would get the following error

Hmm, perhaps we need to decide which version of Python we want to test against by default and then make sure everyone has that available on their machine. (E.g. via conda.)