[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Jelle Zijlstra]

Jelle Zijlstra  added the comment:

I was close to merging the linked PR but there's some renewed concerns about 
the proposed behavior here: Do we really want to change this behavior and 
potentially force a lot of people to change their working code?

In addition, the current code emits FutureWarning. I don't really understand 
what that warning is supposed to be used for 
(https://docs.python.org/3/library/exceptions.html#FutureWarning), but at least 
it means we plan to change the behavior later. So we should have a plan for how 
we'll do that. And whatever we do, the Python and C versions of the code should 
behave consistently.

Here are two ways forward:

(1) We change both the Python and the C versions of ElementTree to emit a 
DeprecationWarning on using __bool__ in 3.11. In 3.13, we change the behavior 
to make __bool__ always return True. (Or perhaps, make it raise an exception.)

(2) We give up on changing this behavior and remove the existing FutureWarning 
from the Python code. But in that case we should document the gotcha in the 
boolean behavior of nodes.

I don't have enough experience with ElementTree to recommend either option, but 
I'm happy to help implement it once we come to a decision.

--
nosy: +JelleZijlstra
versions: +Python 3.11 -Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Guido van Rossum]

Change by Guido van Rossum :


--
nosy:  -gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Jacob Walls]

Change by Jacob Walls :


--
keywords: +patch
nosy: +jacobtylerwalls
nosy_count: 6.0 -> 7.0
pull_requests: +29326
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/31149

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Guido van Rossum]

Guido van Rossum  added the comment:

I doubt effbot will respond. :-)

I think the first order of business is to add the same FutureWarning to the C 
version of the code, and to reset the clock on the deprecation. 

In terms of the final stages, why wouldn't we want it to just always raise? 
Just because there's so much code that might break?

It shouldn't be a SyntaxWarning, since this is most definitely not a syntax 
issue. I'd be okay with RuntimeError (or RuntimeWarning if we decide to keep 
the old outcome with a warning forever).

(Huh, does FutureWarning count as a deprecation?)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Philip Rowlands]

Philip Rowlands  added the comment:

I went digging through the archives, made more interesting as elementtree was 
imported into the standard library.

AFAICT, the FutureWarning for __bool__ (or __nonzero__ in py2) appeared circa 
2007-06 in version 1.3a2:
http://svn.effbot.org/public/tags/elementtree-1.3a3-20070912/CHANGES

- Added future warnings for "if e" (use explicit len(e) or is None
  test) and "e.getchildren()" (use list(e) or iteration).

The docs are still there, warning about the pitfalls and suggesting "This 
behaviour is likely to change somewhat in ElementTree 1.3."
https://effbot.org/zone/element.htm#truth-testing

12 years on, it has not, but what was the intended change (+effbot for possible 
context)??

I assumed on first reading that the plan was to switch bool(Element) to return 
True, but others have pointed out the inconsistency with len(), or having 
__bool__() raise an exception.

My 2c would be to steer devs away from using bool(Element) as a final step, 
i.e. keep the existing True/False definition, warts and all, but raise a 
SyntaxWarning (RuntimeWarning?) with a similar message to the current 
FutureWarning.

--
nosy: +effbot

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Guido van Rossum]

Guido van Rossum  added the comment:

I'm +1 on adding the deprecation and changing it to an error in the future.

I think it's an anti-pattern that objects with complex behavior that includes 
some sequence behavior allow the fallback behavior for checking the empty 
sequence. I recall being bitten by this nearly 20 years ago (at Zope) when some 
database class used this pattern -- somehow the app ended up thinking there was 
no database at all when in fact there was one but it was empty, and it 
reinitialized the database with bad results.

The fallback from truth testing to __len__() is built into the language and 
cannot be changed, but we *can* override __bool__() to raise an exception, and 
I think that's what we should do. (Clearly that's also what we were planning to 
do, hence the warning in the Python version.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Stefan's arguments looked reasonable to me. I supported deprecating 
Element.__bool__. This was an old plan, although we failed with deprecation in 
the C implementation (this is not the first oversight in deprecations in 
ElementTree).

But there is a problem: removing __bool__ will not change the value of 
bool(element). It will still return False for an Element without children, 
because bool() falls back to __len__ if __bool__ is not defined, and 
Element.__len__ is defined. So we will need either to define Element.__bool__ 
always returning True, and this will be the first case when __bool__ and 
__len__ return inconsistent values, or to deprecate __len__ (and perhaps 
__getitem__) that goes too far from the initial plan.

So now I am not sure. Making Element.__bool__ always returning True or raising 
an exception could help to fix some bugs in user code. But this differs from 
common behavior of sequence-like types in Python. On other hand, we have a 
precedence: bool(NotImplemented) raises an exception now (this also can save 
from common errors).

When I started to write this message I was -0 for changing Element.__bool__. 
Now I am +0, after considering all arguments that came in my mind, but still 
have doubts. Can we ask some of the language ideologues?

--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Stefan Behnel]

Stefan Behnel  added the comment:

Not so quick. :) You're probably aware of the details, but still, let me state 
clearly what this is about, to make sure that we're all on the same page here.
(I'm also asking in Serhiy, because he did quite some work on ET in the past 
and has experience with it.)

The Element class in ElementTree represents an XML tag, but it also mimics a 
sequence container of child elements. Its behaviour is quite list-like, in that 
it supports indexing and slicing of its children "list", and also has an 
"append()" method to add one at the end. All of this is a nice and simple API, 
so what's the catch?

The gotcha is the behaviour of "__bool__". Following the sequence protocol, the 
truth value of an Element depends on its children. No children – false. Has 
children, true. While this makes sense from the sequence protocol side, I don't 
think it is what users expect. In Python, it is very common to say "if 
something: …" in order to test if "something" is "a thing". When doing this 
with Elements, users usually don't think of the element's children but the 
Element itself, often because they got it back from from kind of tree search, 
as in the OP's example using ".find()".

I would argue that this is brittle, because it works when testing with Elements 
that happen to have children, but it fails when the Element that was found has 
no children. The intention of the "FutureWarning" was to allow changing the 
behaviour at some point to always return true as the truth value, i.e. to 
remove the "__bool__" implementation all together.

This
1) fixes the behaviour for broken code that intended to test if it "is a thing"
2) does not change the behaviour when testing Elements (for whatever reason) 
that have children
3) returns true instead of false when truth-testing Elements that have no 
children, whether the code originally expected this or not.

The main question now is: what is more common? Code that only accidentally 
works because it didn't get in touch with child-free (leaf-)elements yet? Or 
code that would start failing with this change because it really wants to find 
out if an Element has children by truth-testing it.

I'm sure that both exist, because I have seen both, and my very limited 
*feeling* from what I've seen is that there's more brittle code out there than 
intentional code that truth-tests Elements, and that future users are also more 
likely to get it wrong until they get bitten by it and learn the actual meaning 
of the truth-test.

But, we don't know, and this isn't something to easily find out via a code 
search. I'm personally more leaning towards changing the behaviour to help new 
users, but lacking enough evidence, not changing something long-standing is 
always the safer choice. I'm aware that I will have to take the "final" 
decision in the end, but would be happy to hear some more intuitions, ideas or 
even data points first.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> And, we should finally decide whether this should really be 
> changed or not. The current behaviour is counter-intuitive,
> but it's been like that forever and lots of code depends on 
> it in one way or another, so …

You make a good case to just leave it be.

+1 from me to leave as-is.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Philip Rowlands]

Philip Rowlands  added the comment:

It's easier to justify a change in behaviour if the warning is emitted. With no 
legacy concerns, I would be happy for bool() to change, but I'm not the one who 
would receive the grumbly tickets.

How about emitting the warning in the next release, then assessing the 
behaviour change for version n+2?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Stefan Behnel]

Stefan Behnel  added the comment:

Agreed that both should behave the same.

And, we should finally decide whether this should really be changed or not. The 
current behaviour is counter-intuitive, but it's been like that forever and 
lots of code depends on it in one way or another, so …

--
stage:  -> needs patch
versions: +Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
components: +XML
nosy: +scoder

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38941] xml.etree.ElementTree.Element inconsistent warning for bool

[Philip Rowlands]

New submission from Philip Rowlands :

Steps to reproduce:

$ python3.7
Python 3.7.2 (default, May 13 2019, 13:52:56)
[GCC 6.3.0 20170516] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import xml.etree.ElementTree as ET
>>> bool(ET.fromstring("").find(".//c"))
False


$ python3.7
Python 3.7.2 (default, May 13 2019, 13:52:56)
[GCC 6.3.0 20170516] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.modules['_elementtree'] = None
>>> import xml.etree.ElementTree as ET
>>> bool(ET.fromstring("").find(".//c"))
__main__:1: FutureWarning: The behavior of this method will change in future 
versions.  Use specific 'len(elem)' or 'elem is not None' test instead.


This is (almost) the smallest test case, but in real code what I was trying to 
write was:
```
# check the result code is ok
if response.find("./result[@code='ok']"):
return True
```

The unintuitive bool() behaviour was surprising, compared to other typical True 
/ False determinations on objects, and I think what the FutureWarning is trying 
to avoid.

Please implement the same warning for bool(Element) in _elementtree.c as exists 
in ElementTree.py. bpo29204 was making similar alignments between the versions, 
but didn't consider this FutureWarning.

--
components: Library (Lib)
messages: 357642
nosy: philiprowlands
priority: normal
severity: normal
status: open
title: xml.etree.ElementTree.Element inconsistent warning for bool
type: behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com