-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Booleans specified with -e / --extra-settings do not evaluate to False #2938
Comments
Hi Victor. Many thanks for reporting this problem and posting such a detailed analysis. Much appreciated! 👏 Since you seem to have a good handle on a potential solution, would you consider submitting a PR that addresses the problem, preferably including first a commit with a (failing) test and then another that fixes it? That would really help us out! |
Hi, Justin. Sure, I'll work on it. |
I now realized that To parse a value of some parameter, With default Another observation is that though we allowed non-json booleans like I see two possible solutions:
I'm 100% for the second solution since it's much simpler and predictable. I wouldn't be afraid of breaking code that relies on |
I should note that relying on If you want to accept I'd go with |
Get rid of the coerce_overrides() function. Add the ParseOverrides argparse.Action to parse overrides. Treat all extra settings values strictly as json values. Test overrides. Edit docs and cli help.
Get rid of the coerce_overrides() function. Add the ParseOverrides argparse.Action to parse overrides. Treat all extra settings values strictly as json values. Test overrides. Edit docs and cli help.
Get rid of the coerce_overrides() function. Add the ParseOverrides argparse.Action to parse overrides. Treat all extra settings values strictly as json values. Test overrides. Edit docs and cli help.
Get rid of the coerce_overrides() function. Add the ParseOverrides argparse.Action to parse overrides. Treat all extra settings values strictly as json values. Test overrides. Edit docs and cli help.
pelican-quickstart
pelican-quickstart
Issue
True
by default or inpelicanconf.py
.False
with-e / --extra-settings
.True
.For example, I set
DELETE_OUTPUT_DIRECTORY = True
inpelicanconf.py
and then run:I see how the old files in the
output
directory are deleted. Here's the debug log: https://github.com/r4victor/pelican/blob/boolean_overrides_demo/website/debug.txtSetting
-e param=False
or-e param=0
doesn't work as well. The only way is to set the parameter to an empty string like-e param=
. This is definitely not what users are supposed to do according to the docs (https://docs.getpelican.com/en/latest/settings.html#settings).It seems the problem was introduced when fixing issue #2789 and originates in the
coerce_overrides()
function insettings.py
:So, when
types_to_cast
isbool
, setting-e k=v
results incoerced[k] = bool(v)
, but in Pythonbool(v)
is True for any string except for the empty string.bool("false")
isTrue
.The solution would be to evaluate
"false", "False", "0", ""
(something else?) toFalse
, and everything else toTrue
.The text was updated successfully, but these errors were encountered: