Conversation
DetachHead
left a comment
DetachHead
left a comment
There was a problem hiding this comment.
thanks for the contribution!
public/tach-toml-schema.json
Outdated
| "description": "List of module:name pairs to rename imports (e.g. 'PIL:pillow')" | ||
| }, | ||
| "include_dependency_groups": { | ||
| "type": "boolean", |
There was a problem hiding this comment.
i think we should make this an array of strings. i can't think of many cases where you'd want to include all groups. i know you said this can be added later, but i would prefer if we did it now so we don't need to support two different ways of specifying this setting
if your use case is to include all groups, i think the best approach is to create an all group like the example in the PEP and reference it in your tach config like so:
[external]
include_dependency_groups = ["all"]There was a problem hiding this comment.
To support this, I have changed it to read from pyproject.toml instead of a single global file, to better support the uv workspace monorepo pattern.
There was a problem hiding this comment.
i think that's inconsistent with all the other settings which can be configured in either pyproject.toml or tach.toml. so ideally it would work in both (related: #854)
|
I believe many of these issues should be fixed now! PTAL |
|
|
||
| ### Dependency Groups (PEP 735) | ||
|
|
||
| Tach supports [PEP 735 dependency groups](https://peps.python.org/pep-0735/). By default, Tach will include dependencies from the `dev` dependency group when checking external dependencies. |
There was a problem hiding this comment.
i think we should default to not including any dependency groups. i think in most cases you wouldn't want to import a dev dependency
public/tach-toml-schema.json
Outdated
| "description": "List of module:name pairs to rename imports (e.g. 'PIL:pillow')" | ||
| }, | ||
| "include_dependency_groups": { | ||
| "type": "boolean", |
There was a problem hiding this comment.
i think that's inconsistent with all the other settings which can be configured in either pyproject.toml or tach.toml. so ideally it would work in both (related: #854)
| /// If `include_groups` contains "all", all groups are processed. | ||
| /// Otherwise, only the specified groups are processed. |
There was a problem hiding this comment.
when i suggested an "all" group, i meant that you could define a group called "all" that includes all the other groups, rather than us having to special case the name "all" in tach.
i think if we do want to do this, to avoid ambiguity with a dependency group called "all", we should only allow "all" as a string instead of an item in the array. ie. the python type would be list[str] | Literal["all"].
this will also prevent users from specifying "all" with other dependency groups, which would be redundant
| [project] | ||
| name = "tach" | ||
| version = "0.32.2" | ||
| version = "0.33.0" |
There was a problem hiding this comment.
you can drop this change, i will handle bumping the version myself (there are several spots where it needs to be changed (see #851)
2 participants
References #565
Add basic support for including dependency groups when using
check-external. When the bool is set to true, the set of declared dependencies includes everything in all the dependency groups.This should be a no-op by default as it is
default(false).If desired, later on we can do something more advanced like what the FR requests, but this would be enough for my use right now.