-
Notifications
You must be signed in to change notification settings - Fork 3
Respect inserting pre-commit hooks into a config file following prek syntax #1688
Copy link
Copy link
Closed
Closed
Copy link
Labels
bugSomething isn't workingSomething isn't working
Description
In #1639 I got this stack trace when trying to add a new hook:
Details
╭─────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────╮
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_ui\interface\tool.py:219 in │
│ pre_commit │
│ │
│ 216 │ │ ), │
│ 217 │ │ files_manager(), │
│ 218 │ ): │
│ ❱ 219 │ │ _run_tool(use_pre_commit, remove=remove, how=how) │
│ 220 │
│ 221 │
│ 222 @app.command( │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_ui\interface\tool.py:444 in │
│ _run_tool │
│ │
│ 441 │ from usethis.errors import UsethisError │
│ 442 │ │
│ 443 │ try: │
│ ❱ 444 │ │ caller(remove=remove, how=how, **kwargs) │
│ 445 │ except UsethisError as err: │
│ 446 │ │ err_print(err) │
│ 447 │ │ raise typer.Exit(code=1) from None │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_core\tool.py:191 in │
│ use_pre_commit │
│ │
│ 188 │ │ ensure_dep_declaration_file() │
│ 189 │ │ │
│ 190 │ │ tool.add_dev_deps() │
│ ❱ 191 │ │ _add_all_tools_pre_commit_configs() │
│ 192 │ │ │
│ 193 │ │ for _tool in ALL_TOOLS: │
│ 194 │ │ │ _tool.migrate_config_to_pre_commit() │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_core\tool.py:230 in │
│ _add_all_tools_pre_commit_configs │
│ │
│ 227 │ │ if isinstance(_tool, PreCommitTool): │
│ 228 │ │ │ continue │
│ 229 │ │ if _tool.is_used(): │
│ ❱ 230 │ │ │ _tool.add_pre_commit_config() │
│ 231 │
│ 232 │
│ 233 def use_pyproject_fmt(*, remove: bool = False, how: bool = False) -> None: │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_tool\base.py:175 in │
│ add_pre_commit_config │
│ │
│ 172 │ │ │ │ │ for hook_id in get_hook_ids() │
│ 173 │ │ │ │ ): │
│ 174 │ │ │ │ │ # This will remove the placeholder, if present. │
│ ❱ 175 │ │ │ │ │ add_repo(repo_config) │
│ 176 │ │
│ 177 │ def remove_pre_commit_repo_configs(self) -> None: │
│ 178 │ │ """Remove the tool's pre-commit configuration. │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pre_commit\hooks. │
│ py:106 in add_repo │
│ │
│ 103 │ │ │ predecessor=last_precedent, │
│ 104 │ │ ) │
│ 105 │ │ │
│ ❱ 106 │ │ mgr.commit_model(model) │
│ 107 │
│ 108 │
│ 109 def insert_repo( │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pre_commit\yaml.p │
│ y:59 in commit_model │
│ │
│ 56 │ │
│ 57 │ def commit_model(self, model: schema.JsonSchemaForPreCommitConfigYaml) -> None: │
│ 58 │ │ doc = self.get() │
│ ❱ 59 │ │ dump = _pre_commit_fancy_dump(model, reference=doc.content) │
│ 60 │ │ update_ruamel_yaml_map(doc.content, dump, preserve_comments=True) │
│ 61 │ │ self.model_validate() │
│ 62 │ │ self.commit(doc) │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pre_commit\yaml.p │
│ y:68 in _pre_commit_fancy_dump │
│ │
│ 65 def _pre_commit_fancy_dump( │
│ 66 │ config: schema.JsonSchemaForPreCommitConfigYaml, *, reference: ModelRepresentation │
│ 67 ) -> dict[str, ModelRepresentation]: │
│ ❱ 68 │ dump = fancy_model_dump(config, reference=reference, order_by_cls=ORDER_BY_CLS) │
│ 69 │ │
│ 70 │ if not isinstance(dump, dict): │
│ 71 │ │ msg = ( │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 56 in fancy_model_dump │
│ │
│ 53 │ │ │ model.root, reference=reference, order_by_cls=order_by_cls │
│ 54 │ │ ) │
│ 55 │ elif isinstance(model, BaseModel): │
│ ❱ 56 │ │ return _fancy_model_dump_base_model( │
│ 57 │ │ │ model, reference=reference, order_by_cls=order_by_cls │
│ 58 │ │ ) │
│ 59 │ else: │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 154 in _fancy_model_dump_base_model │
│ │
│ 151 │ │ if display_key is None: │
│ 152 │ │ │ display_key = key │
│ 153 │ │ │
│ ❱ 154 │ │ d[display_key] = fancy_model_dump( │
│ 155 │ │ │ value, reference=value_ref, order_by_cls=order_by_cls │
│ 156 │ │ ) │
│ 157 │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 42 in fancy_model_dump │
│ │
│ 39 │ │ │ │ │ order. RootModels are ignored. │
│ 40 │ """ │
│ 41 │ if isinstance(model, list): │
│ ❱ 42 │ │ return _fancy_model_dump_list( │
│ 43 │ │ │ model, reference=reference, order_by_cls=order_by_cls │
│ 44 │ │ ) │
│ 45 │ elif isinstance(model, dict): │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 88 in _fancy_model_dump_list │
│ │
│ 85 │ │ │
│ 86 │ │ # We don't use None as the fillvalue because it could be confused with the │
│ 87 │ │ # case where the content itself is None. │
│ ❱ 88 │ │ dump = fancy_model_dump(value, reference=ref, order_by_cls=order_by_cls) │
│ 89 │ │ x.append(dump) │
│ 90 │ return x │
│ 91 │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 56 in fancy_model_dump │
│ │
│ 53 │ │ │ model.root, reference=reference, order_by_cls=order_by_cls │
│ 54 │ │ ) │
│ 55 │ elif isinstance(model, BaseModel): │
│ ❱ 56 │ │ return _fancy_model_dump_base_model( │
│ 57 │ │ │ model, reference=reference, order_by_cls=order_by_cls │
│ 58 │ │ ) │
│ 59 │ else: │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 154 in _fancy_model_dump_base_model │
│ │
│ 151 │ │ if display_key is None: │
│ 152 │ │ │ display_key = key │
│ 153 │ │ │
│ ❱ 154 │ │ d[display_key] = fancy_model_dump( │
│ 155 │ │ │ value, reference=value_ref, order_by_cls=order_by_cls │
│ 156 │ │ ) │
│ 157 │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 42 in fancy_model_dump │
│ │
│ 39 │ │ │ │ │ order. RootModels are ignored. │
│ 40 │ """ │
│ 41 │ if isinstance(model, list): │
│ ❱ 42 │ │ return _fancy_model_dump_list( │
│ 43 │ │ │ model, reference=reference, order_by_cls=order_by_cls │
│ 44 │ │ ) │
│ 45 │ elif isinstance(model, dict): │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 88 in _fancy_model_dump_list │
│ │
│ 85 │ │ │
│ 86 │ │ # We don't use None as the fillvalue because it could be confused with the │
│ 87 │ │ # case where the content itself is None. │
│ ❱ 88 │ │ dump = fancy_model_dump(value, reference=ref, order_by_cls=order_by_cls) │
│ 89 │ │ x.append(dump) │
│ 90 │ return x │
│ 91 │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 56 in fancy_model_dump │
│ │
│ 53 │ │ │ model.root, reference=reference, order_by_cls=order_by_cls │
│ 54 │ │ ) │
│ 55 │ elif isinstance(model, BaseModel): │
│ ❱ 56 │ │ return _fancy_model_dump_base_model( │
│ 57 │ │ │ model, reference=reference, order_by_cls=order_by_cls │
│ 58 │ │ ) │
│ 59 │ else: │
│ │
│ C:\Users\namc\AppData\Local\uv\cache\archive-v0\yW5imlSdi7A20ot9JLrcF\Lib\site-packages\usethis\_integrations\pydantic\dump.py: │
│ 129 in _fancy_model_dump_base_model │
│ │
│ 126 │ │
│ 127 │ d: dict[str, ModelRepresentation] = {} │
│ 128 │ for key, value in model: │
│ ❱ 129 │ │ default_value = model.__class__.model_fields[key].default │
│ 130 │ │ │
│ 131 │ │ # The value for the reference (for recursion) │
│ 132 │ │ value_ref = _get_value_ref(reference, key=key) │
╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
KeyError: 'priority'
I think this is because our pre-commit file includes extra fields like priority: 0 which are defined in the prek config schema but not the original pre-commit. However, we should respect them and not error out with KeyError in this case. Let's add a test for this kind of YAML file in our pre-commit integration tests.
We should ideally respect arbitrary keys - not only priority.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working