Repository navigation
kernels-data: strongly type version length - #794
Merged
Merged
Conversation
`Version` stored versions in a dynamic-length `Vec` and would trim any trailing zeros (e.g. `13.0` -> `13`). This leads to issues when we re-serialize `build.toml` files where we actually meant `13.0` (e.g. for Nix version comparisons, `13.0` and `13` are different). We could simply remove the zero-trimming logic, but this also does not seem correct, because a user could e.g. use 13.0.0 for a CUDA version, but this breaks some Nix logic as well and that is not the grannularity we want to use for e.g. CUDA and Torch versions. So instead, add the version length as a type parameter, e.g. `Version<2>`. This ensures that we are always serializing correctly and catches erroneous version use by the user (e.g `13` or `13.0.0` when `N=2`). Also add some tests.
danieldk
changed the title
danieldk
marked this pull request as draft
August 28, 2026 09:07
Reject bad data.
Coverage report —
|
| Name | Stmts | Miss | Cover | Missing |
|---|---|---|---|---|
| src/kernels/__init__.py | 14 | 0 | 100% | |
| src/kernels/_system.py | 6 | 1 | 83% | 10 |
| src/kernels/_versions.py | 78 | 9 | 88% | 47, 53-54, 57-58, 97, 119, 130, 136 |
| src/kernels/backends.py | 212 | 62 | 71% | 40, 44, 48-51, 68, 90, 108, 117, 121, 125-127, 148, 157, 161, 165-167, 188, 199, 201, 208-211, 224, 228, 232-252, 260, 283-303 |
| src/kernels/compat.py | 8 | 1 | 88% | 5 |
| src/kernels/deps.py | 67 | 1 | 99% | 57 |
| src/kernels/hf_hub.py | 62 | 3 | 95% | 18, 20, 114 |
| src/kernels/importer.py | 50 | 3 | 94% | 110, 114, 117 |
| src/kernels/install.py | 21 | 7 | 67% | 78-102 |
| src/kernels/layer/__init__.py | 6 | 0 | 100% | |
| src/kernels/layer/_interval_tree.py | 103 | 4 | 96% | 23, 52, 147, 150 |
| src/kernels/layer/device.py | 48 | 14 | 71% | 42, 47-49, 91, 96-98, 101, 149, 152, 155-157 |
| src/kernels/layer/func.py | 86 | 6 | 93% | 90, 121, 193, 313, 338, 366 |
| src/kernels/layer/globals.py | 5 | 0 | 100% | |
| src/kernels/layer/kernelize.py | 74 | 8 | 89% | 255, 281, 289-290, 296, 300, 316-318 |
| src/kernels/layer/layer.py | 215 | 15 | 93% | 177, 222, 247, 365, 445-446, 458, 467, 475, 486, 515, 519, 532, 585, 615 |
| src/kernels/layer/mode.py | 14 | 0 | 100% | |
| src/kernels/layer/repos.py | 144 | 42 | 71% | 27, 33, 36-43, 63-64, 70, 73-76, 90, 94, 103-104, 110, 113-116, 123-124, 130, 133-136, 143-144, 150, 153-156, 163-164, 170, 173-176, 257 |
| src/kernels/load.py | 61 | 2 | 97% | 297, 335 |
| src/kernels/locking.py | 89 | 64 | 28% | 35-83, 91-98, 102-125, 137, 152-159, 165-175, 179-186 |
| src/kernels/python_deps.py | 58 | 6 | 90% | 59-60, 64-65, 101, 104 |
| src/kernels/resolver.py | 135 | 2 | 99% | 174, 180 |
| src/kernels/status.py | 49 | 2 | 96% | 23, 81 |
| src/kernels/variants.py | 278 | 19 | 93% | 64, 95, 116, 146, 255-256, 298-301, 303, 387-394, 400-406, 437-443, 455-461 |
| src/kernels/verify.py | 88 | 1 | 99% | 32 |
| TOTAL | 1971 | 272 | 86% |
Updated by the Test kernels workflow on commit 28b4105f2bf3db11b142cbaff71b8d821a73eb82.
danieldk
marked this pull request as ready for review
August 28, 2026 09:32
sayakpaul
approved these changes
Aug 28, 2026
Comment on lines
+7
to
+9
| assert str(Version.from_str("12.8.0")) == "12.8" | ||
| assert str(Version.from_str("1")) == "1" | ||
| assert str(Version.from_str("1.2.3")) == "1.2.3" |
Member
There was a problem hiding this comment.
One for 13.0 as well since it came up quite a bit? 😁
Member
Author
There was a problem hiding this comment.
😁 but not in a Python context though.
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Versionstored versions in a dynamic-lengthVecand would trim any trailing zeros (e.g.13.0->13). This leads to issues when we re-serializebuild.tomlfiles where we actually meant13.0(e.g. for Nix version comparisons,13.0and13are different).We could simply remove the zero-trimming logic, but this also does not seem correct, because a user could e.g. use 13.0.0 for a CUDA version, but this breaks some Nix logic as well and that is not the granularity we want to use for e.g. CUDA and Torch versions.
So instead, add the version length as a type parameter, e.g.
Version<2>. This ensures that we are always serializing correctly and catches erroneous version use by the user (e.g13or13.0.0whenN=2).For the Python
Versiontype, we retain the existing behavior in order to not break kernels for now that do not have akernels-dataupper pin. However, we store the version asBox<[usize]>, so that we can use the same Python class if we want to embed Rust typed versions in the future (since we can return a slice forVersion<N>).Fixes #792.