https://pantsbuild.org/ logo
Join Slack
Powered by
Are dict option values expected to be sorted? I ca...
# development
b
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
I wouldn't have expected them to be. Where are you seeing the sorting, 
./pants help
output?
b
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
ah - a possible bug?
b
I guess it all depends on what people would intuit about a dictionary option 🤷‍♂️
I could see it either way.
h
probably more how dict options are being consumed. If order does impact plugin behavior, then our caching is wrongg
b
I guess it doesn't today, but I was trying to introduce something where it does 😅
h
fwiw, fixing eq/hash isn't an API change imo. It's only caching less than before, which seems safe
b
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
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
Yeah makes sense
OK I'll PR it
c
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
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
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
In this case, it'd specifically be options
c
can’t they be in a list, tho?
b
Option keys are always strings
c
do you have any code/example to share?
b
(That may be a limitation of the (new) option code, and not the parser however)
c
maybe time to dust of the idea of option schemas 😅
b
I woulnd't mind that... being done by someone else 😅
c
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
So why didn't you use ordered dicts for the rule stuff? My ignorant vision says that'd be a really nice UX?
p
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
Benjy talked me into it 😄
p
@happy-kitchen-89482 care to change your mind? 😄
b
Wanna make a new thread for that? I need to circle back to options 😛
c
noooo 😛
b
Looks like the parser can parse dicts-with-tuple-keys
c
yep, that’s what it was..
b
Although... that doesn't help the order thing
p
That (dict w/ tuple keys) is used for 
__defaults__
.
✅ 1
b
(Sorry, should've realized that sooner)
@proud-dentist-22844 I'm talking about options, not targets 😉
p
ah. pants.toml
✅ 1
c
but dicts does preserve ordering, right? until there’s a funny dict class that applies sorted to your elements, then you’re toast 😛
b
It preserves order in iteration, not in equality
c
I mean, it’s a thin ice, easy to break..
p
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
ooh good razor
c
do you have example toml config for this, Josh?
b
Hmm Although @proud-dentist-22844 the spec might not have an answer as it doesn't say anything about evaluation of the values.
p
In yaml, maps are explicitly NOT ordered. But in practice they are in python because of python dicts.
b
Key/value pairs within tables are not guaranteed to be in any specific order.
👍 1
p
That’s your answer then.
➕ 1
b
Excellent idea to check the spec @proud-dentist-22844 🙂
Although for the targets debate, y'all still CAN used ordered 😉
c
h
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
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
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
☝️ 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
Open in Slack
PreviousNext
Powered by