Add `remove` feature for dict options

[Eric-Arellano]

We now have several dict-basd options, particularly the resolves options for JVM and Python. remove is useful to avoid having to duplicate what was previously set.

@benjyw ponders in https://github.com/pantsbuild/pants/pull/16481#discussion_r943625332 what this should look like. I personally think the list of key names makes sense and is the simplest solution.

[benjyw]

Adding a list of key names in general is not so simple. It means that for env vars and flags we now have to support something like: "-['x','y'],+{'z':42}".

I propose instead that we have a special value symbol representing a removal, so e.g.:

--foo="{'x': '__remove_me__', 'y': '__remove_me__', 'z': 42}"

Then the parsing remains identical to today, and there is just some post-processing.

Note that:

A) We can possibly spell '__remove_me__' as None in the stringified dict literal, since I don't think None ever makes sense as a value in an option dict. I'm not sure what would happen today if we tried to use one but I will try. TOML has no None equivalent, but it does have a NaN that we could use similarly.

B) We can support a remove list of names specifically in config files, because those get processed entirely within config.py and turned into '__remove_me__' style removes before being passed on to the rest of the options system.

[Eric-Arellano]

--foo="{'x': 'remove_me', 'y': 'remove_me', 'z': 42}"

That syntax results in overriding, though, right? You need to use a - or + prefix to not override.

[benjyw]

Er yeah, there would be a + prefix in front of that whole thing.

--foo="+{'x': '__remove_me__', 'y': '__remove_me__', 'z': 42}"

It looks better as:

--foo="+{'x': None 'y': None, 'z': 42}"

[stuhood]

Adding a list of key names in general is not so simple. It means that for env vars and flags we now have to support something like: "-['x','y'],+{'z':42}".

This still seems simpler to me than adding an additional syntax affordance... for one thing, both -nan and None are valid dict entries (although the former is maybe a bit ridiculous).

[benjyw]

Simpler in what way? The implementation isn't simpler, at any rate.

I'm not convinced None is ever useful as an option dict value in practice. In fact, I'm not sure we even support them except possibly by accident. We declare a dict opt using, e.g., DictOption[str] (or infer the value type from the default). And None is a NoneType, not a str. So you get errors like ValueError: Values for log_domain_levels must be strings, but was given the value: None with type <class 'NoneType'>.

Update: I guess that error is specifically on the consumer side for this specific option. So we do pass None through that far, so it could be used in other contexts as a valid value. Still seems shady to me.

[benjyw]

The nan thing is annoying, but you live by toml, you die by toml...

[stuhood]

Simpler in what way? The implementation isn't simpler, at any rate.

Conceptually, I very much expect that folks will look for a remove option for dicts if they are already familiar with lists: it's not a new concept. They won't find it (or will get an opaque error about remove not existing), and then they will definitely need to look at the docs to figure out what to do.

But on the other hand, if a user uses a "dict removal" (-{..}) rather than a "list removal" (-[..]), we can render a helpful error message: "To remove items from a dict, use -[..] for the keys you'd like to remove".