-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
python modernize 2to3 tool fix_filter #23454
python modernize 2to3 tool fix_filter #23454
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23454/4993 |
A new Pull Request was created by @davidlange6 (David Lange) for master. It involves the following packages: Alignment/MillePedeAlignmentAlgorithm @kpedro88, @fabozzi, @nsmith-, @rekovic, @thomreis, @vanbesien, @arizzi, @perrotta, @civanch, @monttj, @cmsbuild, @GurpreetSinghChahal, @davidlange6, @smuzaffar, @Dr15Jones, @mdhildreth, @jfernan2, @cerminar, @slava77, @ggovi, @fabiocos, @prebello, @vazzolini, @kmaeshima, @arunhep, @dmitrijus, @franzoni, @gpetruc, @lpernie can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@@ -289,15 +289,15 @@ def sinkPorts(self): | |||
return [port for port in self._ports if port.portType() == "sink"] | |||
def isSink(port): | |||
return port.portType() == 'sink' | |||
return filter(isSink, self._ports) | |||
return list(filter(isSink, self._ports)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we understand this change? Does filter
return a generator in python 3 while the old filter
returned a list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so now I understand you consider this a problem. not sure what alternatives there are. I'll have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems an easy replacement is
return [item for item in self._ports if isSink(item)]
I'd propose to handle the 13 changes (mostly in obsolete code and all in non-performance critical code) for a followup pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concur.
Right.
https://stackoverflow.com/questions/41666977/python-2-vs-python-3-difference-in-behavior-of-filter
… On Jun 4, 2018, at 10:18 AM, Chris Jones ***@***.***> wrote:
@Dr15Jones commented on this pull request.
In FWCore/GuiBrowsers/python/Vispa/Gui/ConnectableWidget.py:
> @@ -289,15 +289,15 @@ def sinkPorts(self):
return [port for port in self._ports if port.portType() == "sink"]
def isSink(port):
return port.portType() == 'sink'
- return filter(isSink, self._ports)
+ return list(filter(isSink, self._ports))
Do we understand this change? Does filter return a generator in python 3 while the old filter returned a list?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
please test |
The tests are being triggered in jenkins. |
+1 |
+1 |
+1
|
+1 |
+1 |
+operations the expanded output of the Configuration/DataProcessing unit test is unchanged |
+1 |
+1 |
+1 |
merge |
apply fixes.fix_filter tool from python futurize tool