https://pantsbuild.org/ logo
Join Slack
Powered by
What do folks think about removing the implicit `d...
# development
b
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
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
I think changing the implicit default might be a much more painful and invasive change šŸ˜•
c
Ok, so perhaps I didnā€™t quite understand what you propose with ā€œremoving the `default`ā€ .. ? šŸ¤Ŗ
b
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
Ah, right. OK, that makes sense.
b
(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
TBH that feels like it might be very disruptive, and break a lot of external plugins
b
@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
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
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
All the options have to be registered before we parse cmd line flags
Is the problem
b
Sure, all I'm saying is I would only expect perf to stay the same, with the ability to maybe get better. Never worse
Open in Slack
PreviousNext
Powered by