Title: Implement `usethis tool pre-commit` · Issue #22 · usethis-python/usethis-python · GitHub
Open Graph Title: Implement `usethis tool pre-commit` · Issue #22 · usethis-python/usethis-python
X Title: Implement `usethis tool pre-commit` · Issue #22 · usethis-python/usethis-python
Description: Motivation pre-commit is an essential framework. It will interact with many other tools so it is better to implement usethis tool pre-commit sooner rather than later. Summary of desired feature usethis tool pre-commit should add, install...
Open Graph Description: Motivation pre-commit is an essential framework. It will interact with many other tools so it is better to implement usethis tool pre-commit sooner rather than later. Summary of desired feature use...
X Description: Motivation pre-commit is an essential framework. It will interact with many other tools so it is better to implement usethis tool pre-commit sooner rather than later. Summary of desired feature use...
Opengraph URL: https://github.com/usethis-python/usethis-python/issues/22
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Implement `usethis tool pre-commit`","articleBody":"**Motivation**\r\n[pre-commit](https://pre-commit.com/) is an essential framework. It will interact with many other tools so it is better to implement `usethis tool pre-commit` sooner rather than later.\r\n\r\n**Summary of desired feature**\r\n`usethis tool pre-commit` should add, install and configure `pre-commit`.\r\nBy default, it will add a single, simple hook, however also any other tools which have been added using `usethis` should be detected and configured as hooks too; at the moment it is just `deptry`.\r\n\r\n**Design**\r\nWe can add `pre-commit` as a dev dependency using `uv`, with a message.\r\n\r\nEvery tool provided by `usethis` will potentially write to configuration files. Usually, this will be `pyproject.toml` but for `pre-commit` hooks, it will be `.pre-commit-config.yaml`.\r\n\r\nAny time we come to add to a `.pre-commit-config.yaml` file, if it does not already exist, then we can start with a simple file with one example hook and display a message:\r\n```YAML\r\nrepos:\r\n - repo: https://github.com/abravalheri/validate-pyproject\r\n rev: \"v0.19\"\r\n hooks:\r\n - id: validate-pyproject\r\n additional_dependencies: [\"validate-pyproject-schema-store[all]\"]\r\n```\r\nThen we can run `uv run pre-commit autoupdate` in a subprocess to update the `rev` tag to the latest verison. If offline, this won't work so we'll just leave the hard-coded version (e.g. as `v0.19`), although for now let's not worry about handling the case of being offline. There will be a chore associated with bumping the `rev` in each release of `usethis` - we can ensure this with a version check test.\r\n\r\nWe should create a `Protocol` class for each tool; implementations of the protocol should provide a method which gives the contents of the relevant config files. In general, this might depend on other things that are installed, or existing configuration.\r\nAnother part of the protocol should be a method to determine whether the tool is being used in a given repo. This might involve some heuristics but to begin with we will just check whether the tool is a dev dependency (for the `pre-commit` and `deptry` tools, which are the only ones so far). Here is what I expect the classes would roughly look like:\r\n\r\n```Python\r\nclass Tool(Protocol):\r\n def get_pre_commit_repo_config(self) -\u003e PreCommitRepoConfig:\r\n raise NotImplementedError\r\n \r\n def add_pre_commit_repo_config(self) -\u003e None:\r\n #TODO add the pre-commit config, creating the config file if necessary, and deleting any old config first.\r\n ...\r\n \r\n @abstractmethod\r\n def is_used(self) -\u003e bool: ...\r\n\r\nclass DeptryTool(Tool):\r\n def get_pre_commit_repo_config(self) -\u003e PreCommitRepoConfig:\r\n # TODO deptry pre-commit config\r\n ...\r\n \r\n def is_used(self) -\u003e bool:\r\n # TODO for now just check whether deptry is a dev dependency in the pyproject.toml file\r\n ...\r\n\r\nclass PreCommitTool(Tool):\r\n def is_used(self) -\u003e bool:\r\n # TODO for now just check whether pre-commit is a dev dependency in the pyproject.toml file\r\n ...\r\n```\r\n\r\nWe should loop over tools, Following the protocol, we can use the method to determine whether `get_pre_commit_repo_config` raises `NotImplementedError`: if so, we don't need to add pre-commit hooks, otherwise, we do. This should be done with a function which takes a `PreCommitRepoConfig` and adds it. This function can call another function which creates the `.pre-commit-config.yaml` file, if it does not already exist, as described above.\r\n\r\nTo add the new `deptry` hook, we need to remove any hook that already has the ID `deptry`, and replace it with the new one, including a message. This ensures that any older version of `usethis` or any repo configured with a different methodology will still be compatible with the current version of `usethis`. There should be a function which removes a hook with a given ID. Again, this can be called the hook-adding function.\r\n\r\nWe will use `ruamel.yaml` to preserve round-trip formatting, comments, etc. of the YAML file.\r\n\r\nThere are different ways to set up the `deptry` pre-commit hook, but currently, I believe the best way something like this:\r\n```YAML\r\n - repo: local\r\n hooks:\r\n - id: deptry\r\n name: deptry\r\n entry: uv run --frozen deptry src\r\n language: system\r\n always_run: true\r\n pass_filenames: false\r\n```\r\nThere are a variety of reasons for this choice, explained later.\r\n\r\nWe should have a canonical sort order for all `usethis`-supported hooks to decide where to place the section. The main objective with the sort order is to ensure dependency relationships are satisfied. For example, `valdiate-pyproject` will check if the `pyproject.toml` is valid - if it isn't then some later tools might fail. It would be better to catch this earlier. A general principle is to move from the simpler hooks to the more complicated. Of course, this order might already be violated, or the config might include unrecognized repos - in any case, we aim to ensure the new tool is configured correctly, so it should be placed after the last of its precedents. This logic can be encoded in the adding function.\r\n\r\nOnce we have added the hooks, we will then want to try `install pre-commit` from a subprocess; if we're not in a git repo, this will fail, in which case, we should fail hard with a descriptive error message (eventually we might support `usethis tool git` under the hood and manage setup automatically instead of giving up). Otherwise give a message that pre-commit has been installed.\r\n\r\nAs a part of all these changes, we should also change the `usethis tool deptry` command - if using `pre-commit` (again, the tool protocol should provide a method to detect this) we should again add the pre-commit configuration for `deptry`.\r\n\r\nOne issue will be determining the directory in which the source code is found. I am of the opinion that `src` should be a default/pre-requisite for `usethis`. Alternatively, this could be some kind of global usethis configuration. For now, I think let's hard-code it and fail hard if `src` is not a directory, with a helpful message. There are potentially other directories such as `tests` and `doc` which would benefit from checking, but `deptry` [does not yet support](https://github.com/fpgmaas/deptry/issues/302) checking of folders where dev dependencies are allowed. This check should be enforced as a part of the `usethis tool deptry` command.\r\n\r\n**Describe alternatives you've considered**\r\n\r\n- Rather than a `Protocol` class which is config-file oriented, we might try and create an abstraction which doesn't give `pre-commit` special first-class status and just models _inter-tool_ interactions (possibly between more than 2 tools) and the objects would define the interaction effects (which might be saving a particular piece of config in a file, or perhaps something else). I still haven't ruled something like this out but the abstraction isn't clear to me yet, and I don't think it would be too hard to refactor later. Also, interactions between tools is unusual for the most part. The protocol approach seems simple and appropriate at this stage of development.\r\n- `ruamel.yaml` seems like the only option in terms of round-trip modification of YAML; `pyyaml` would not be powerful enough.\r\n- Rather than including the hook for `validate-pyproject`, we might include a different hook (or hooks). One option was maximum file size (but that might disrupt a user of Git LFS). Ultimately it's basically just a placeholder and can be changed later. It is very unlikely that someone wouldn't want this sort of valdiation to their `pyproject.toml` file. We take for granted that a `pyproject.toml` file is being used for `uv add` to work, so this is a reasonable choice.\r\n- Rather than running `uv run pre-commit autoupdate` to get the version of `validate-pyproject`, we might just rely on the hard-coded version. This would mean less complexity and maybe a performance improvement would mean that an outdated version of usethis would give an outdated version of `validate-pyproject`. On the balance, the user experience of getting the latest version is more important.\r\n\r\nIt's worth going into some detail as to why the `deptry` hook is set up the way it is. Configuring deptry with pre-commit is a little tricky since it needs to access the virtual environment to automatically determine the names of modules (used in import statements) exposed by each distribution package (i.e. installed with `uv`/`pip`/etc.). As such, a `system` hook is really the only option to get the necessary dependencies without explicitly enumerating them in the `.pre-commit-config.yaml` file. This is approach taken by [the officially supported hook](https://github.com/fpgmaas/deptry/blob/main/.pre-commit-hooks.yaml); as a `system` hook. However, that particular official hook doesn't really work in my experience when running while committing rather than manually from the CLI, since the `system` hook is not able to activate the virtual environment. Whereas using `uv run` will activate the venv. Also, with `uv run`, all the packages are all based on the lockfile, including `deptry` itself, which is useful since `deptry` otherwise there is a version syncing issue.\r\n\r\nThis is a more general issue: how to handle synchronization between dev dependencies' versions and the corresponding versions in the pre-commit-config.yaml file; also how to ensure that the syncing is not disrupted by `pre-commit.ci`'s automatic update requests.\r\nIn the long run, I think the best approach is to use something like `sync-pre-commit-lock` but [this is still waiting on `uv` support](https://github.com/GabDug/sync-pre-commit-lock/issues/42). `deptry` will not benefit from this approach. Since [you can't exclude specific repos](https://github.com/pre-commit/pre-commit/issues/1959) from the `autoupdate `command, we would need to use `[pre-commit-update](https://gitlab.com/vojko.pribudic.foss/pre-commit-update)` and this would be incompatible with using the autoupdate provided by `pre-commit.ci`, which is a shame. An alternative is just to remove the dev dependency and rely on `pre-commit`, which is fine for some tools (e.g. `validate-pyproject`) which are only ever really used in the context of `pre-commit`, but for things like `ruff` which have IDE integrations, etc. it's not an option. So the `uv run` local hook is a very attractive workaround in the interim until we work through the complexity of doing `pre-commit` version updates; not just for `deptry` but for other tools too.\r\n\r\nIt would be theoretically possible to use a `python` language hook and make `uv` a dependency, but then `pre-commit.ci` will try to run it by default, so we would have to introduce complexity to configure the `ci` section for support by `pre-commit.ci`, etc. Better to use a `system` hook and just assume `uv` is installed, which we are doing anyway.\r\n\r\nWe could fail hard if we're not in a git repo, but this probably means there needs to be complexity associated with checking this up-front - it's potentially easier to just add all the configuration and then rely on pre-commit itself to determine whether installation is successful (which should hopefully be equivalent to whether `git` is installed).\r\n\r\n**Testing Strategy**\r\n\r\nJust calling `usethis tool pre-commit` when there is a git repo and basic, valid `pyproject.toml` file, and no `.pre-commit-config.yaml`` file:\r\n- Test `pre-commit` is added as a dev dependency\r\n- Test the config file gets created\r\n- Test the config file contains the expected contents, including hard-coded version for validate-pyproject which will need bumping\r\n- Test that calling `pre-commit run --all-files` from a subprocess with invalid TOML in the `pyproject.toml` file fails.\r\n- Test that calling `pre-commit run --all-files` from a subprocess with valid TOML in the `pyproject.toml` file runs successfully\r\n- Test that trying to commit invalid TOML to the `pyproject.toml` file is rejected by the pre-commit hook\r\n- Test that trying to commit valid TOML to the `pyproject.toml` file is accepted by the pre-commit hook.\r\n\r\nTest the case where the `pre-commit-config.yaml` file already exists (and git repo, and `pyproject.toml`), that there is no error raised. \r\n\r\nWhen calling `usethis tool pre-commit` when there isn't a git repo (but is a `pyproject.toml` file), test the command fails.\r\nWhen calling `usethis tool pre-commit` when there isn't a `pyproject.toml` file (but is a git repo), test the command fails.\r\n\r\nTest we can call the CLI for `usethis tool pre-commit` from a subprocess.\r\n\r\nCalling `usethis tool deptry` and then `usethis tool pre-commit` in succession (and vice versa; test cases should be the same):\r\n- Test that calling `pre-commit run deptry --all-files` from a subprocess succeeds.\r\n- Test that calling `pre-commit run deptry --all-files` from a subprocess with bad dependency relationships fails.\r\n\r\nTest all the output messages are correct (potentially just within multiple of the tests above).\r\n\r\n**Steps**\r\n\r\n1. Write an empty `def pre_commit() -\u003e None` function.\r\n2. Test `pre-commit` is added as a dev dependency.\r\n3. Call `uv add` from a subprocess in the `pre_commit` function.\r\n4. Test a message regarding the new dev dependency.\r\n5. Add the message.\r\n6. Test the `.pre-commit-config.yaml` file exists.\r\n7. In the `pre_commit` function, add an empty `.pre-commit-config.yaml` file if it does not already exist.\r\n8. Test a message regarding the new config file.\r\n9. Add the message.\r\n10. Test that the `.pre-commit-config.yaml` file has the expected contents, including latest version of `validate-pyproject`.\r\n11. Modify the `.pre-commit-config.yaml` population logic so that it doesn't write empty contents but instead writes the expected contents, followed by `pre-commit autoupdate` from a subprocess.\r\n12. Add all the remaining tests and pass them.\r\n\r\n**Acceptance Criteria**\r\nAssuming `uv` and `git` are installed, it should be possible to call `usethis tool pre-commit` from the command-line in a new project, and then immediately call `pre-commit run --all-files` to run the pre-commit hooks for any tools managed by `usethis`, e.g. `deptry`.","author":{"url":"https://github.com/nathanjmcdougall","@type":"Person","name":"nathanjmcdougall"},"datePublished":"2024-10-05T03:03:51.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":3},"url":"https://github.com/22/usethis-python/issues/22"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:1ad9d6c9-8b00-d73d-7a7c-cd928ff93199 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | 9DA0:1CB805:117665F:181C3E2:6A4DDF98 |
| html-safe-nonce | f65f20078ca42b09735b9c78ba914b55e7fd912a72a9b2fd2faefe5cc52fb923 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI5REEwOjFDQjgwNToxMTc2NjVGOjE4MUMzRTI6NkE0RERGOTgiLCJ2aXNpdG9yX2lkIjoiNjI1MDMyNDU2MDI3ODU3NzA0OCIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | 3dfb1b0e967c48b71c80982d038ff831b182d97a2d14b1b057917f7dc14be9d3 |
| hovercard-subject-tag | issue:2567616324 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/usethis-python/usethis-python/22/issue_layout |
| twitter:image | https://opengraph.githubassets.com/fea891cab43b0e98f088cded2815c389599760581a40a822bb262fec267667c4/usethis-python/usethis-python/issues/22 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/fea891cab43b0e98f088cded2815c389599760581a40a822bb262fec267667c4/usethis-python/usethis-python/issues/22 |
| og:image:alt | Motivation pre-commit is an essential framework. It will interact with many other tools so it is better to implement usethis tool pre-commit sooner rather than later. Summary of desired feature use... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | nathanjmcdougall |
| hostname | github.com |
| expected-hostname | github.com |
| None | 06b8a6144231bf3a234f1c2e9993861e07ce98a905912b114aa386c2d7e84b33 |
| turbo-cache-control | no-preview |
| go-import | github.com/usethis-python/usethis-python git https://github.com/usethis-python/usethis-python.git |
| octolytics-dimension-user_id | 216362695 |
| octolytics-dimension-user_login | usethis-python |
| octolytics-dimension-repository_id | 842189705 |
| octolytics-dimension-repository_nwo | usethis-python/usethis-python |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 842189705 |
| octolytics-dimension-repository_network_root_nwo | usethis-python/usethis-python |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 1d344bdb7547fe6bca17a59bb2b8aac3dc9532a0 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width