https://pantsbuild.org/ logo
b

bitter-ability-32190

11/29/2022, 6:28 PM
Are dict option values expected to be sorted? I can't find any documentation saying as such, but the behavior I'm seeing is that they are
h

hundreds-father-404

11/29/2022, 6:28 PM
I wouldn't have expected them to be. Where are you seeing the sorting,
./pants help
output?
b

bitter-ability-32190

11/29/2022, 6:29 PM
printing them in a integration test directly from my subsystem in a rule
Passing the value on the CLI. I see it being passed in a certain order, but printed always in a sorted order. The options parsing code isn't quite grokkable, but I also don't see explitly see any sorting?
So
DictValueComponent.create
is preserving order...
Ahhhh so this is interesting. order IS preserved, but the value is eq/hash based on contents and not order, so changing the order doesn't invalidate the cache.
👍 1
h

hundreds-father-404

11/29/2022, 6:44 PM
ah - a possible bug?
b

bitter-ability-32190

11/29/2022, 6:44 PM
I guess it all depends on what people would intuit about a dictionary option 🤷‍♂️
I could see it either way.
h

hundreds-father-404

11/29/2022, 6:46 PM
probably more how dict options are being consumed. If order does impact plugin behavior, then our caching is wrongg
b

bitter-ability-32190

11/29/2022, 6:46 PM
I guess it doesn't today, but I was trying to introduce something where it does 😅
h

hundreds-father-404

11/29/2022, 6:47 PM
fwiw, fixing eq/hash isn't an API change imo. It's only caching less than before, which seems safe
b

bitter-ability-32190

11/29/2022, 6:48 PM
True, although I think the value is truly a
dict
, so that means we'd need to swap that with a new dict type. Which sounds nasty?
Equality tests between
OrderedDict
objects are order-sensitive
But maybe not that hard 😉
h

hundreds-father-404

11/29/2022, 6:49 PM
FrozenDict
I thinkkkkk considers order iirc
Note that DictOption was created when Pants still ran with Python 2.7 and dict order was not preserved. So, historically, this is all correct. It's only new that the order can be leveraged by plugin authors
b

bitter-ability-32190

11/29/2022, 6:50 PM
Yeah makes sense
OK I'll PR it
c

curved-television-6568

11/29/2022, 6:50 PM
I think order of dict entries should not be significant.. that would be surprising in my book at least 😛
i.e. dict with same contents in different order yields eq hashes
b

bitter-ability-32190

11/29/2022, 6:51 PM
In this case, if you don't care, then nothing changes. But if you do care...
In my case, I'm defining a mapping from glob -> thing. I need to specify what happens if multiple globs match. First one wins? Last one wins? Unspecified? It'd be nice if it was first or last.
c

curved-television-6568

11/29/2022, 6:53 PM
aahhh… that’s the reason why the dep rule globs are in a list rather than a dict (as I had it starting out) but changed after lengthy discussion with @happy-kitchen-89482 on the topic 😄
b

bitter-ability-32190

11/29/2022, 6:53 PM
In this case, it'd specifically be options
c

curved-television-6568

11/29/2022, 6:54 PM
can’t they be in a list, tho?
b

bitter-ability-32190

11/29/2022, 6:54 PM
Option keys are always strings
c

curved-television-6568

11/29/2022, 6:55 PM
do you have any code/example to share?
b

bitter-ability-32190

11/29/2022, 6:55 PM
(That may be a limitation of the (new) option code, and not the parser however)
c

curved-television-6568

11/29/2022, 6:55 PM
maybe time to dust of the idea of option schemas 😅
b

bitter-ability-32190

11/29/2022, 6:55 PM
I woulnd't mind that... being done by someone else 😅
c

curved-television-6568

11/29/2022, 6:56 PM
lol
well, in that case, I’m not going to block you if you need to make a change wrgt how dicts are treated, just aired my view on the matter 😉
b

bitter-ability-32190

11/29/2022, 6:58 PM
So why didn't you use ordered dicts for the rule stuff? My ignorant vision says that'd be a really nice UX?
p

proud-dentist-22844

11/29/2022, 7:00 PM
I think the dep rule globs would be nicer in a dict - the n-tuple construction is a little surprising since there is a clear map between key and value.
c

curved-television-6568

11/29/2022, 7:00 PM
Benjy talked me into it 😄
p

proud-dentist-22844

11/29/2022, 7:00 PM
@happy-kitchen-89482 care to change your mind? 😄
b

bitter-ability-32190

11/29/2022, 7:01 PM
Wanna make a new thread for that? I need to circle back to options 😛
c

curved-television-6568

11/29/2022, 7:01 PM
noooo 😛
b

bitter-ability-32190

11/29/2022, 7:01 PM
Looks like the parser can parse dicts-with-tuple-keys
c

curved-television-6568

11/29/2022, 7:01 PM
yep, that’s what it was..
b

bitter-ability-32190

11/29/2022, 7:01 PM
Although... that doesn't help the order thing
p

proud-dentist-22844

11/29/2022, 7:01 PM
That (dict w/ tuple keys) is used for
__defaults__
.
1
b

bitter-ability-32190

11/29/2022, 7:02 PM
(Sorry, should've realized that sooner)
@proud-dentist-22844 I'm talking about options, not targets 😉
p

proud-dentist-22844

11/29/2022, 7:02 PM
ah. pants.toml
1
c

curved-television-6568

11/29/2022, 7:03 PM
but dicts does preserve ordering, right? until there’s a funny dict class that applies sorted to your elements, then you’re toast 😛
b

bitter-ability-32190

11/29/2022, 7:03 PM
It preserves order in iteration, not in equality
c

curved-television-6568

11/29/2022, 7:03 PM
I mean, it’s a thin ice, easy to break..
p

proud-dentist-22844

11/29/2022, 7:03 PM
I think in this case, we’d need to consult the toml spec - are dicts supposed to be ordered? or is that a side-effect of using python dicts?
💯 1
1
b

bitter-ability-32190

11/29/2022, 7:04 PM
ooh good razor
c

curved-television-6568

11/29/2022, 7:04 PM
do you have example toml config for this, Josh?
b

bitter-ability-32190

11/29/2022, 7:05 PM
Hmm Although @proud-dentist-22844 the spec might not have an answer as it doesn't say anything about evaluation of the values.
p

proud-dentist-22844

11/29/2022, 7:05 PM
In yaml, maps are explicitly NOT ordered. But in practice they are in python because of python dicts.
b

bitter-ability-32190

11/29/2022, 7:05 PM
Key/value pairs within tables are not guaranteed to be in any specific order.
👍 1
p

proud-dentist-22844

11/29/2022, 7:06 PM
That’s your answer then.
1
b

bitter-ability-32190

11/29/2022, 7:08 PM
Excellent idea to check the spec @proud-dentist-22844 🙂
Although for the targets debate, y'all still CAN used ordered 😉
c

curved-television-6568

11/29/2022, 7:09 PM
h

happy-kitchen-89482

11/29/2022, 7:34 PM
I didn’t want the rules to be a dict because order matters
and relying on order in a dict is subtle and extremely easy to get wrong
b

bitter-ability-32190

11/29/2022, 7:36 PM
As of now, the versions of Python which don't support ordering are no longer active 🙂
I think it's OK, honestly. It makes sense lexically (as we humans like), is much nicer on the eyes, and handles duplicate keys 😉
h

happy-kitchen-89482

11/29/2022, 7:39 PM
I’m still skeptical, although if we convert from a dict to a list at parse time and keep it as a list internally, I could maybe be convinced
But it would still seem to the casual reader that order is not significant
b

bitter-ability-32190

11/29/2022, 7:41 PM
☝️ I'm not sure I agree with that. BUILD files are Python, and Python has insertion-ordered dicts
Oh hey @happy-kitchen-89482 I think we're on the wrong thread. This one was wrapped up
🤦‍♂️ 1