Fix squished zip key ordering #1815
Conversation
mbargull
force-pushed
the
fix-squished-zip_key-ordering
branch
from
3f3dd4e to
22fc34c
Compare
|
Is there a test we could include? Admittedly this may be tricky |
|
@mbargull said on the call just now that the code above is not correct and needs changes |
Values from top level keys with possibly (as in hash seed-dependent) different ordering overwrote some of the determined used variants. If values of related zipped keys were left untouched, this led to mismatched ordering of zip_keys entries. ref: - conda-forge#1459 (comment) Signed-off-by: Marcel Bargull <[email protected]>
mbargull
force-pushed
the
fix-squished-zip_key-ordering
branch
from
22fc34c to
85482fd
Compare
|
A test here is going to be pretty hard to make. |
| # NOTE: "pin_run_as_build" (dict of dict) order is not handled. | ||
| # "tuple" below only used for uniform list/tuple/str comparison. | ||
| v_set = set(map(tuple, v)) | ||
| v = type(input_variant)( | ||
| v_i for v_i in input_variant if tuple(v_i) in v_set | ||
| ) |
There was a problem hiding this comment.
I opted to no handle pin_run_as_build since its order should not matter, i.e., we'd had to do special handling for that case here with no further benefit.
I added at least a note so we are aware that it's intentionally omitted.
The variant entries should be of type str, so a straight v_set = set(v) would be possible.
We do, however, also have zip_keys which is a list of list/tuple.
To handle potential tuple-to-list comparisons (and to get hashable types for list entries) this just casts everything to tuples.
Feel free to amend any of this in case you'd want more explicit handling.
Tried this on
Yeah, at least we have https://github.com/conda-forge/conda-smithy/blob/v3.30.4/tests/test_cli.py#L90 (which was actually tripped with the first pass I had with this). |
|
I'll wait a day or so and then merge if things are looking good. Thank you! |
Makes sense. Thanks for reviewing! |
Checklist
newsentryFixes gh-1459
Fixes the
conda-smithypart affecting gh-1782 (that issue needs further adjustments to the recipe and/orconda-build).gh-1459 is an old issue such that the initial discussion refers to old code.
A recent analysis regarding current code is at #1459 (comment) .
This PR adds the suggested change from #1459 (comment) .
With that change the issue from conda-forge/python-feedstock#656 (comment) has not been reproducible for me (for a selection of
PYTHONHASHSEEDvalues).Similarly the issue from conda-forge/pytensor-suite-feedstock#54 reported here at gh-1782 does not reproduce for me (when fixing top-level
pyselector without{{ python }}usage in that recipe).