https://pantsbuild.org/ logo
#development
Title
# development
b

bitter-ability-32190

02/24/2022, 2:36 PM
What do folks think about removing the implicit
default
for "scalar" option types (like
str
,
bool
,
int
) etc... in the new
XOption
descriptor classes? The reasoning is that (especially for bool, although also for others) right now the implicit default of
None
isn't obvious. Users might expect a
StrOption
to have a default of
""
, etc...
It also simplifies the options code 😀
Only 6 files (8 options) need to be changed in the repo. One of which was a
BoolOption
c

curved-television-6568

02/24/2022, 2:41 PM
Unless I got your suggestion wrong, how about changing the implicit default from
None
to
scalar()
so, you get
""
for
str
and
0
for
int
etc, so we can’t still provide an explicit default when applicable.
Or, maybe what I’m asking for is the ability to mark an option as “optional”, that includes a way to discern a missing value from the default “empty” value.
b

bitter-ability-32190

02/24/2022, 2:44 PM
I think changing the implicit default might be a much more painful and invasive change 😕
c

curved-television-6568

02/24/2022, 2:47 PM
Ok, so perhaps I didn’t quite understand what you propose with “removing the `default`” .. ? 🤪
b

bitter-ability-32190

02/24/2022, 2:47 PM
Oops, yeah re-reading it's ambiguous one sec
OK, it's removing the implicit default (of
None
) in favor of making clients pass
None
themselves
c

curved-television-6568

02/24/2022, 2:48 PM
Ah, right. OK, that makes sense.
b

bitter-ability-32190

02/24/2022, 2:57 PM
(Although it would seem
mypy
can't be taught how to handle this.
pyright
handles it right out of the box, pun intended)
It's a shame the Python-native typechecker is (IMO) consistently being outshined by a checker written in TS which requires
npm
to run 😠
h

happy-kitchen-89482

02/24/2022, 3:13 PM
TBH that feels like it might be very disruptive, and break a lot of external plugins
b

bitter-ability-32190

02/24/2022, 3:18 PM
@happy-kitchen-89482 for the record, this would be in the new Options descriptor classes. So likely breaking no one 🙂
(I should've been more explicit)
(and another thing
mypy
doesnt support but others do,
typing_extensions.Self
)
h

happy-kitchen-89482

02/24/2022, 3:58 PM
Ah, I see
Then my only concern would be about inconsistency, but since in the long run I imagine wanting to move all options to the new system...
1
With the caveat that we'd have to do some benchmarking first, because in Pants v1 option registration took a measurable amount of time on startup
so we'd want to be sure the new registration mechanism isn't dramatically slower
b

bitter-ability-32190

02/24/2022, 4:05 PM
I would expect with concrete types, we have the ability to improve perf. As we'll be able to "scope" code to the relevant class
h

happy-kitchen-89482

02/24/2022, 4:06 PM
All the options have to be registered before we parse cmd line flags
Is the problem
b

bitter-ability-32190

02/24/2022, 4:07 PM
Sure, all I'm saying is I would only expect perf to stay the same, with the ability to maybe get better. Never worse