bpo-31648: Improve ElementPath #3835

scoder · 2017-09-30T08:31:12Z

https://bugs.python.org/issue31648

…he current node, like "[.='text']".

serhiy-storchaka

Add a What's New entry. Document the new feature and add a "versionchanged" directive in the documentation.

serhiy-storchaka · 2017-09-30T08:44:08Z

Misc/NEWS.d/next/Library/2017-09-30-10-45-12.bpo-31648.Cai7ji.rst

@@ -0,0 +1,3 @@
+Improvements to path predicates in ElementTree:
+* Allow whitespace around predicate parts, i.e. "[a = 'text']" instead of requiring the less readable "[a='text']".
+* Add support for text comparison of the current node, like "[.='text']".


Add "Patch by Stefan Behnel".

serhiy-storchaka · 2017-09-30T08:51:01Z

Lib/test/test_xml_etree.py

+            ['section'])
+        self.assertEqual(summarize_list(e.findall(".//section[tag = 'subtext']")),
+            ['section'])
+        self.assertEqual(summarize_list(e.findall(".//section[ tag = 'subtext' ]")),


Wouldn't be enough to test just this variant?

Whitebox testing, yes. Blackbox testing, no. It's short enough, so I chose the latter. In parsers, it's easy to accidentally forget about special cases.

serhiy-storchaka · 2017-09-30T08:51:12Z

Lib/test/test_xml_etree.py

+                         ['tag'])
+        self.assertEqual(summarize_list(e.findall('.//tag[.= "subtext"]')),
+                         ['tag'])
+        self.assertEqual(summarize_list(e.findall('.//tag[ . = "subtext" ]')),


Wouldn't be enough to test just this variant?

Same comment applies.

serhiy-storchaka · 2017-09-30T08:58:35Z

Lib/xml/etree/ElementPath.py

+        else:
+            def select(context, result):
+                for elem in result:
+                    if "".join(elem.itertext()) == value:
                        yield elem
                        break


I think there shouldn't be a break. Add a test for this.

This is inside of a predicate, so we only care about "found" or "not found". The break was there before.

The break was there before for breaking an internal loop. findall() should find all elements, not the first one.

Ah, you're right. :)

bedevere-bot · 2017-09-30T08:59:49Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I didn't expect the Spanish Inquisition!. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

scoder · 2017-09-30T09:21:07Z

Documentation updated.

serhiy-storchaka · 2017-09-30T09:49:21Z

Doc/whatsnew/3.7.rst

+ElementPath predicates in the :meth:`find` methods can now compare text
+of the current node with ``[. = "text"]``, not only text in children.
+Predicates also allow adding spaces for better readability.
+


Add "(Contributed by ...)" as in other entries.

serhiy-storchaka · 2017-09-30T09:51:14Z

Lib/xml/etree/ElementPath.py

@@ -157,6 +157,8 @@ def prepare_predicate(next, token):
            return
        if token[0] == "]":
            break
+        if token == ('', ''):


Is it for whitespaces? Please add a comment.

serhiy-storchaka · 2017-09-30T09:52:01Z

Lib/xml/etree/ElementPath.py

                        yield elem
-                        break


Please add a special test case for this bug.

Ah, it is already added!

Already done.

serhiy-storchaka · 2017-09-30T09:59:07Z

Doc/whatsnew/3.7.rst

+ElementPath predicates in the :meth:`find` methods can now compare text
+of the current node with ``[. = "text"]``, not only text in children.
+Predicates also allow adding spaces for better readability.
+(Contributed by Stefan Behnel.)


Add a reference to the issue.

serhiy-storchaka · 2017-09-30T10:04:22Z

Misc/NEWS.d/next/Library/2017-09-30-10-45-12.bpo-31648.Cai7ji.rst

@@ -0,0 +1,4 @@
+Improvements to path predicates in ElementTree:
+* Allow whitespace around predicate parts, i.e. "[a = 'text']" instead of requiring the less readable "[a='text']".


These lines are merged together in the generated HTML. Add empty lines before and after the enumeration list.

scoder · 2017-09-30T10:11:22Z

Updated.

serhiy-storchaka · 2017-09-30T10:13:42Z

Misc/NEWS.d/next/Library/2017-09-30-10-45-12.bpo-31648.Cai7ji.rst

 * Allow whitespace around predicate parts, i.e. "[a = 'text']" instead of requiring the less readable "[a='text']".
+


I think this one is redundant.

serhiy-storchaka · 2017-09-30T10:22:38Z

Doc/whatsnew/3.7.rst

+xml.etree
+---------
+
+ElementPath predicates in the :meth:`find` methods can now compare text


Make ElementPath a link:

:class:`~xml.etree.ElementPath`

Add a reference to elementtree-xpath:

:ref:`some text <elementtree-xpath>`

I added a reference.

scoder · 2017-09-30T11:18:17Z

Updated the news entries.
If you have further ideas about doc changes, I think it might be faster for you to edit the files, instead of explaining what changes you would like to have.

Stefan Behnel added 2 commits September 30, 2017 10:20

bpo-31648: Allow whitespace inside of ElementPath predicates.

d5ce7ac

bpo-31648: Add ElementPath predicate support for text comparison of t…

3d4826b

…he current node, like "[.='text']".

bedevere-bot added the awaiting review label Sep 30, 2017

the-knights-who-say-ni added the CLA signed label Sep 30, 2017

scoder changed the title ~~Improve ElementPath~~ bpo-31648: Improve ElementPath Sep 30, 2017

serhiy-storchaka reviewed Sep 30, 2017

View reviewed changes

serhiy-storchaka requested changes Sep 30, 2017

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Sep 30, 2017

Add news entry.

8de5ec5

scoder force-pushed the elementpath branch from 5e1a954 to 8de5ec5 Compare September 30, 2017 09:09

bpo-31648: Update documentation.

a3078f5

bpo-31648: Remove left-over "break" that prevents finding all matches.

782ae97

serhiy-storchaka reviewed Sep 30, 2017

View reviewed changes

scoder added 2 commits September 30, 2017 11:54

Update what's new entry.

29bbbcb

bpo-31648: Add comment.

4879c72

serhiy-storchaka reviewed Sep 30, 2017

View reviewed changes

scoder added 2 commits September 30, 2017 12:07

bpo-31648: Add issue reference to what's new entry.

26be846

Reformat news entry.

6442ee3

serhiy-storchaka reviewed Sep 30, 2017

View reviewed changes

scoder added 2 commits September 30, 2017 13:11

Reformat news entry.

86fb63c

bpo-31648: Add context reference to what's new entry.

140749f

serhiy-storchaka approved these changes Sep 30, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Sep 30, 2017

serhiy-storchaka added the type-feature A feature request or enhancement label Sep 30, 2017

serhiy-storchaka merged commit 101a5e8 into python:master Sep 30, 2017

Mariatta removed the awaiting merge label Oct 8, 2017

		@@ -0,0 +1,4 @@
		Improvements to path predicates in ElementTree:
		* Allow whitespace around predicate parts, i.e. "[a = 'text']" instead of requiring the less readable "[a='text']".

Uh oh!

bpo-31648: Improve ElementPath #3835

bpo-31648: Improve ElementPath #3835

Uh oh!

Conversation

scoder commented Sep 30, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Sep 30, 2017

Uh oh!

scoder commented Sep 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scoder commented Sep 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scoder commented Sep 30, 2017

Uh oh!

Uh oh!

scoder commented Sep 30, 2017 •

edited by bedevere-bot

Loading