Closed jugmac00 closed 3 years ago
d-maurer
commented
3 years ago Jürgen Gmach wrote at 2020-10-12 23:01 -0700:
...
What actually happened:
Traceback (innermost last): Module Shared.DC.ZRDB.Search, line 57, in manage_addZSearch TypeError: object of type 'map' has no len()
This is a Python 3 incompatibility: under Python 3, map
no longer returns a list but an interator (and you cannot call
len on an iterator). Fix: enclose the map call in list(...).
jugmac00
commented
3 years ago Right! That and str vs bytes were the most common problems when migrating from Python 2 to 3.
I am not a fan of doing code changes without a proper test, and this week I am very short on time.
I'd probably wait on feedback by the original poster in https://community.plone.org/t/error-when-i-want-create-zsearchinterface/12985 - maybe the poster would like to introduce a fix?
jugmac00
commented
3 years ago modernize is a tool to help transform Python 2 code to Python 3.
That is the output for Search.py
+from __future__ import absolute_import
+from six.moves import map
try:
from html import escape # noqa
except ImportError:
@@ -44,7 +46,7 @@
if input_title and not input_id:
raise ValueError('No <em>input id</em> were specified')
- qs = map(lambda q, self=self: _getquery(self, q), queries)
+ qs = list(map(lambda q, self=self: _getquery(self, q), queries))
arguments = {}
keys = []
@@ -180,7 +182,7 @@
def default_input_form(arguments, action='query',
tabs=''):
if arguments:
- items = arguments.items()
+ items = list(arguments.items())
return (
'%s\n%s%s' % (
'<html><head><title><dtml-var title_or_id></title>'
@@ -191,9 +193,7 @@
'<table>\n'
% (tabs, action),
'\n'.join(
- map(
- lambda a:
- ('<tr><th>%s</th>\n'
+ [('<tr><th>%s</th>\n'
' <td><input name="%s"\n'
' size="30" value="%s">'
'</td></tr>'
@@ -201,7 +201,7 @@
'type' in a[1]
and ('%s:%s' % (a[0], a[1]['type'])) or a[0],
'default' in a[1] and a[1]['default'] or '',
- )), items)),
+ )) for a in items]),
'\n<tr><td colspan=2 align=center>\n'
'<input type="SUBMIT" name="SUBMIT" value="Submit Query">\n'
'</td></tr>\n</table>\n</form>\n'
@@ -220,7 +220,7 @@
def default_input_zpt_form(arguments, action='query', tabs=''):
if arguments:
- items = arguments.items()
+ items = list(arguments.items())
return (
'%s\n%s%s' % (
'<html><body>\n%s\n'
@@ -230,9 +230,7 @@
'<table>\n'
% (tabs, action),
'\n'.join(
- map(
- lambda a:
- ('<tr><th>%s</th>\n'
+ [('<tr><th>%s</th>\n'
' <td><input name="%s"\n'
' size="30" value="%s">'
'</td></tr>'
@@ -240,7 +238,7 @@
'type' in a[1]
and ('%s:%s' % (a[0], a[1]['type'])) or a[0],
'default' in a[1] and a[1]['default'] or '',
- )), items)),
+ )) for a in items]),
'\n<tr><td colspan=2 align=center>\n'
'<input type="SUBMIT" name="SUBMIT" value="Submit Query">\n'
'</td></tr>\n</table>\n</form>\n'
RefactoringTool: Files that need to be modified:
RefactoringTool: src/Shared/DC/ZRDB/Search.pySo, there are more things to update for Python 3 compatibility.
dataflake
commented
3 years ago I am -1 on just blindly accepting all changes from running some tool over the code. It is not necessary to wrap all occurrences of certain operations (map, keys, values, items, etc) in a list call. And why is from __future__ import absolute_import needed here? Only fix what breaks.
jugmac00
commented
3 years ago I know you are skeptic of tools changing code, I just wanted to offer a possible solution.
I used it for migrating my Zope app, and @icemac used it for afaik the huge Union CMS ( see his video https://www.youtube.com/watch?v=O-xSmvQwPHY&ab_channel=EuroPythonConference ).
It is true - this is a static code analyzer, so it cannot decide always whether list is necessary or not. It just shows potential problems. The above diff is printed to stdout by default, so it does not change any code.
It is always a question of how much time do I want to invest and do I trust some tool? I err on the side of trusting tools, though reviewing the changes afterwards.
The reasoning about from __future__ import absolute_import derives from https://www.python.org/dev/peps/pep-0328/#id5 , second paragraph. On a high level it shows potential problems in Python 2 code, which you can fix before porting to Python 3.
Once again, it might not be necessary here. But it does not hurt.
When migrating my Zope app to Python 3, I added this very line to every module and so prevented dozens of possible runtime errors for untested modules.
For completeness sake, once I switch completely to Python 3, I add pyupgrade to my linters, and so unnecessary future imports get deleted automatically.
This all said - this is a small module, possibly not used a lot, otherwise the Python 3 bug would have been reported much earlier. The mentioned tooling makes much more sense for larger scales, and even there it is a subjective decision on whether to use it or not.
dataflake
commented
3 years ago I know you want to offer a solution and there is no need to explain the rationale why you use a tool somewhere. It's still better to fix specifically what's broken. Coercing stuff into a list can carry a high cost of the iterator is large. And "it does not hurt" is not an argument for anything, sorry.
BUG/PROBLEM REPORT (OR OTHER COMMON ISSUE)
What I did:
I tried to use
Z Search Interfacevia ZMI.What I expect to happen:
I expected to get a generated template with a search form.
What actually happened:
Looks like this code path was not tested for Python 3.
What version of Python and Zope/Addons I am using:
Zope 4.5.0 Python 3.7.4 Products.ZSQLMethods 3.11
Additional Info
This is the first time I tried to use this function - I did this in order to support the following question on the Plone forum: https://community.plone.org/t/error-when-i-want-create-zsearchinterface/12985
The broken code has no coverage