Opened 5 years ago
Closed 18 months ago
#16082 closed enhancement (wontfix)
Python3 : Test ticket for zip()
Reported by:  wluebbe  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  python3  Keywords:  python3, days85 
Cc:  embray, tscrim, jmantysalo, jdemeyer, jhpalmieri  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/16082 (Commits)  Commit:  8795f70fbad9a3e3cbbdd329e071020062d0b5be 
Dependencies:  Stopgaps: 
Description (last modified by )
This ticket is used for doctesting the Python 3 compatibility of zip()
. It is not a real ticket and should not be merged.
Change History (37)
comment:1 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:2 Changed 5 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:3 Changed 3 years ago by
 Component changed from distribution to python3
comment:4 Changed 3 years ago by
 Milestone changed from sage6.4 to sage7.4
comment:5 Changed 3 years ago by
comment:6 Changed 3 years ago by
 Milestone changed from sage7.4 to sageduplicate/invalid/wontfix
 Status changed from new to needs_review
as of sage 7.5.rc2
running all doctests with "from future_builtins import zip" does not give any error
I propose therefore to close this ticket
EDIT: this needs to be checked very carefully.
comment:7 Changed 2 years ago by
Could you elaborate on what you meant by "This needs to be checked very carefully."
comment:8 Changed 2 years ago by
Yes, I can.
What I mean is the following: one should find a way to run all doctests with the py3 behaviour of zip activated both in doctests and in code. I have not been able to do that so far, as it seems that adding an import line to src/sage/all.py is not enough.
Same thing for range in #16081
comment:9 Changed 2 years ago by
 Branch set to public/16082
 Commit set to 737c6682235caded1b7d809db84b8951016a4bbf
Here is a test branch, displaying the issue : how to trigger future zip behaviour in globally in the code ?
New commits:
737c668  a test branch for zip, but not working as desired

comment:10 Changed 2 years ago by
Could you patch globals()['__builtins__'].zip
in sage.all
?
comment:11 followup: ↓ 12 Changed 2 years ago by
 Status changed from needs_review to needs_info
You should really think about putting from six.moves import zip
in sage.all
. That will affect everybody using Sage. Doing something like that should be explicitly discussed on sagedevel (and not in a thread called "how to check zip, map, range, print for Python3").
comment:12 in reply to: ↑ 11 ; followup: ↓ 14 Changed 2 years ago by
Replying to jdemeyer:
You should really think about putting
from six.moves import zip
insage.all
. That will affect everybody using Sage. Doing something like that should be explicitly discussed on sagedevel (and not in a thread called "how to check zip, map, range, print for Python3").
This is a *f#### test ticket*, with milestone wontfix. It has never been my intention to make this enter sage. Could you please make a more useful contribution ?
comment:13 Changed 2 years ago by
 Description modified (diff)
 Status changed from needs_info to needs_review
 Summary changed from Python 3 preparation: The semantics of the zip() function is changed to Test ticket for zip()
Was it really necessary to be so unpolite?
It seems that the only indication that it was a test ticket is the milestone. Nothing in the title nor the description nor in your sagedevel thread was making it clear that this was a test ticket. So that could easily be overlooked.
comment:14 in reply to: ↑ 12 Changed 2 years ago by
Replying to chapoton:
Could you please make a more useful contribution ?
After being insulted like that? Why would I?
comment:15 Changed 2 years ago by
Sorry for my rough reaction. All my apologies. Early morning's bad mood.
Could you please, given all that you know that I don't, tell if you think that the goal of this ticket is reachable ?
comment:16 Changed 2 years ago by
 Commit changed from 737c6682235caded1b7d809db84b8951016a4bbf to 8795f70fbad9a3e3cbbdd329e071020062d0b5be
comment:17 Changed 2 years ago by
ok, this works and discovers a lot of issues: two notable ones being
for op,i in reversed(zip(ef, indices)): TypeError: argument to reversed() must be a sequence
return Factorization(zip(pols, exps), leading_coeff) File "/home/u1/chapoton/sage/local/lib/python2.7/sitepackages/sage/structure/factorization.py", line 294, in __init__ raise TypeError("x must be a list")
comment:18 Changed 2 years ago by
see #22271 for a partial cure
comment:19 Changed 2 years ago by
see #22350 for the next pill in the cure
comment:20 Changed 2 years ago by
then #22385
comment:21 Changed 2 years ago by
then #22457
comment:22 Changed 2 years ago by
then #22519
comment:23 Changed 2 years ago by
 Status changed from needs_review to needs_work
I find it always confusing that this "needs review". Can we leave it as "needs work" until all the zip problems are fixed?
comment:24 Changed 2 years ago by
They are now fixed, or should be. This really needs review now (but I admit it was set to needs review before reaching this state).
comment:25 Changed 2 years ago by
 Keywords days85 added
comment:26 Changed 2 years ago by
So is the question here now whether the patchbot is happy?
comment:27 Changed 2 years ago by
Yes, exactly. And sadly it seems not to be the case. At least this one failure seems related and needs investigation:
sage t long warnlong 75.6 src/sage/graphs/generic_graph.py ********************************************************************** File "src/sage/graphs/generic_graph.py", line 6558, in sage.graphs.generic_graph.GenericGraph.longest_path Failed example: g.longest_path(algorithm="backtrack").edges() Expected: [(0, 1, None), (1, 2, None), (2, 3, None), (3, 4, None), (4, 9, None), (5, 7, None), (5, 8, None), (6, 8, None), (6, 9, None)] Got: []
comment:28 Changed 2 years ago by
see #22603 for maybe the last step
comment:29 Changed 2 years ago by
 Status changed from needs_work to needs_review
green bot, which means that all zip problems have been taken care of
comment:30 Changed 2 years ago by
 Summary changed from Test ticket for zip() to Python3 : Test ticket for zip()
comment:31 Changed 2 years ago by
 Cc tscrim jmantysalo jdemeyer jhpalmieri added
can we please close this as duplicate, now that everything is in order ?
comment:32 Changed 2 years ago by
Do you want to keep this ticket around as a general test ticket against regressions or do you have another methodology for that?
comment:33 Changed 2 years ago by
I have no other methodology. Maybe it is better to keep it open so that patchbots can be launched on it. The patchbots should now be very reluctant to test closed tickets indeed.
comment:34 Changed 2 years ago by
 Status changed from needs_review to needs_info
so let us rather put that into needs_info status
comment:35 Changed 18 months ago by
 Status changed from needs_info to positive_review
I now propose to close this one. If somebody ever reintroduces a bad zip, we will see that when running all doctests with python3.
comment:37 Changed 18 months ago by
 Resolution set to wontfix
 Status changed from positive_review to closed
one step in #21568