py-thinc: added versions 8.1.10 and 8.3.10; and increased readability of complex dependencies#3397
py-thinc: added versions 8.1.10 and 8.3.10; and increased readability of complex dependencies#3397qwertos wants to merge 5 commits intospack:developfrom
Conversation
tldahlgren
left a comment
tldahlgren
left a comment
There was a problem hiding this comment.
Confirmed the new version sha256s and most dependencies. (Note there are newer (patch) releases.)
It appears there's a missing dependency on py-ml-datasets@0.2 (see https://github.com/explosion/thinc/blob/v8.3.x/requirements.txt#L10 and https://github.com/explosion/thinc/blob/v8.1.10/requirements.txt#L10).
Ideally dependencies would be listed with those for newer versions appearing first, and not only for readability. (See comment.)
See comments for more suggestions.
|
|
||
| depends_on("py-numpy@1.7:", when="@:7.4.1") | ||
| depends_on("py-numpy@1.15:", when="@8.1.10:") | ||
| depends_on("py-numpy@2:", when="@8.1.10:") |
There was a problem hiding this comment.
Where is this coming from for 8.1.10? I'm not seeing a requirement for py-numpy@2:.
But I do see it for 8.3.x: https://github.com/explosion/thinc/blob/v8.3.x/pyproject.toml#L9
There was a problem hiding this comment.
Good catch here! Will fix.
| depends_on("py-plac@0.9.6:1.1", type=("build", "run")) | ||
| depends_on("py-tqdm@4.10:4", type=("build", "run")) | ||
|
|
||
| depends_on("py-cython@0.25:2", type="build", when="@8.1.10") |
There was a problem hiding this comment.
| depends_on("py-cython@0.25:2", type="build", when="@8.1.10") | |
| depends_on("py-cython@0.25:2", type="build", when="@8.1.10:") |
There was a problem hiding this comment.
This change would seem to conflict with line 35. Can you explain what is intended here?
|
|
||
| with default_args(type=("build", "run")): | ||
| depends_on("py-blis@0.4.0:0.4", when="@:7.4.1") | ||
| depends_on("py-blis@0.7.8:0.7", when="@8.1.10") |
There was a problem hiding this comment.
| depends_on("py-blis@0.7.8:0.7", when="@8.1.10") | |
| depends_on("py-blis@0.7.8:0.7", when="@8.1.10:") |
There was a problem hiding this comment.
This change would seem to conflict with line 40. Can you explain what is intended here?
| depends_on("py-numpy@1.15:", when="@8.1.10:") | ||
| depends_on("py-numpy@2:", when="@8.1.10:") |
There was a problem hiding this comment.
There are some python version-dependent lower bounds for py-numpy in, e.g., https://github.com/explosion/thinc/blob/v8.1.12/pyproject.toml#L9.
There was a problem hiding this comment.
Good catch here! Will fix.
| depends_on("py-numpy@1.15:", when="@8.1.10:") | ||
| depends_on("py-numpy@2:", when="@8.1.10:") | ||
|
|
||
| depends_on("py-pydantic@1.7.4:1.7,1.9:1.10", when="@8.1.10") |
There was a problem hiding this comment.
| depends_on("py-pydantic@1.7.4:1.7,1.9:1.10", when="@8.1.10") | |
| depends_on("py-pydantic@1.7.4:1.7,1.9:1.10", when="@8.1.10:") |
There was a problem hiding this comment.
This change would seem to conflict with line 66. Can you explain what is intended here?
There was a problem hiding this comment.
There isn't a problem with overlap. The concretizer handles it.
What adding the colon allows is it ensures the dependency applies should someone decide (someday) to add a patch release between it and the next minor release.
There was a problem hiding this comment.
AFAIK that's only true if the dependency version ranges don't have an upper bound. Here, the concretizer would have to find a version of py-pydantic that is both <= 1.10 and of major version 2; an impossible task.
There was a problem hiding this comment.
I thought your question was why I suggested appending the colon to the when condition. If someone were to add the version 8.1.12 then, without the colon or any other changes here, this package would not have a dependency on py-pydantic. In other words, with the colon, there would not be a need to make any changes to the py-pydantic dependency.
There was a problem hiding this comment.
Maybe I could do something like @8.1.10:8.1 but I would not feel comfortable making an intentional declaration like that without going through all published versions within that range and confirming it's accurate. It very quickly can get to the point where I may as well add every version (at least the versions between the oldest and newest version() directives) published on pypi since I'd be doing that work anyway. At the end of the day, IMHO it's the responsibility of the person who would add version("8.1.12") to make sure that the depends_on() directives are accurate for that version. And if those dependencies get missed, that's the value of having peer checkers.
2 participants
No description provided.