Opened 23 months ago
Closed 23 months ago
#27346 closed enhancement (fixed)
py3: fix FindStat interface and FancyTuple
Reported by:  mantepse  Owned by:  

Priority:  major  Milestone:  sage8.7 
Component:  python3  Keywords:  
Cc:  chapoton, embray  Merged in:  
Authors:  Martin Rubey  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  488e74e (Commits)  Commit:  488e74e8ddfa36757ca0479ea540778e8338f8b9 
Dependencies:  Stopgaps: 
Description (last modified by )
various little fixes here and there...
Change History (17)
comment:1 Changed 23 months ago by
 Branch set to u/mantepse/py3__fix_findstat_interface_and_fancytuple
comment:2 Changed 23 months ago by
 Cc chapoton added
 Commit set to 66931ff6189650c6b7c8a145978340dd24d627d2
 Component changed from PLEASE CHANGE to python3
 Description modified (diff)
 Status changed from new to needs_review
 Type changed from PLEASE CHANGE to enhancement
comment:3 followup: ↓ 6 Changed 23 months ago by
 Cc embray added
This is wrong:
+ if sys.version_info[0] < 3 and isinstance(result, unicode):
The "unicode" keyword cannot be used in code that is compatible with both py2 and py3. And one should refrain from using sys.version_info everywhere.
comment:4 followup: ↓ 5 Changed 23 months ago by
Why is this ticket needed at all ? All doctests pass with py3, if I am not mistaken.
comment:5 in reply to: ↑ 4 Changed 23 months ago by
Replying to chapoton:
Why is this ticket needed at all ? All doctests pass with py3, if I am not mistaken.
Unfortunately, not at all:
martin@convex63:~/sagepython3$ ./sage t optional=sage,internet src/sage/databases/findstat.py too many failed tests, not using stored timings Running doctests with ID 20190225104409a715d7b7. Git branch: develop Using optional=internet,memlimit,sage Doctesting 1 file. sage t src/sage/databases/findstat.py ... 28 items had failures: 11 of 16 in sage.databases.findstat 6 of 11 in sage.databases.findstat.FindStat 4 of 10 in sage.databases.findstat.FindStat.__call__ 2 of 6 in sage.databases.findstat.FindStatCollection 1 of 3 in sage.databases.findstat.FindStatCollections.__iter__ 7 of 9 in sage.databases.findstat.FindStatCollections._element_constructor_ 2 of 3 in sage.databases.findstat.FindStatMap.description 2 of 3 in sage.databases.findstat.FindStatMap.id 2 of 3 in sage.databases.findstat.FindStatMap.id_str 2 of 3 in sage.databases.findstat.FindStatMap.name 1 of 3 in sage.databases.findstat.FindStatMaps 3 of 5 in sage.databases.findstat.FindStatStatistic.__eq__ 2 of 4 in sage.databases.findstat.FindStatStatistic.__getitem__ 1 of 3 in sage.databases.findstat.FindStatStatistic.__init__ 3 of 4 in sage.databases.findstat.FindStatStatistic.__ne__ 2 of 4 in sage.databases.findstat.FindStatStatistic.__repr__ 1 of 9 in sage.databases.findstat.FindStatStatistic._find_by_values 4 of 6 in sage.databases.findstat.FindStatStatistic._generating_functions_dict 3 of 8 in sage.databases.findstat.FindStatStatistic._raise_error_modifying_statistic_with_perfect_match 1 of 3 in sage.databases.findstat.FindStatStatistic.data 3 of 5 in sage.databases.findstat.FindStatStatistic.first_terms 1 of 2 in sage.databases.findstat.FindStatStatistic.function 1 of 2 in sage.databases.findstat.FindStatStatistic.references 3 of 4 in sage.databases.findstat.FindStatStatistic.set_code 5 of 6 in sage.databases.findstat.FindStatStatistic.set_description 3 of 4 in sage.databases.findstat.FindStatStatistic.set_references 3 of 4 in sage.databases.findstat.FindStatStatistic.set_sage_code 1 of 2 in sage.databases.findstat.FindStatStatistic.submit [247 tests, 80 failures, 16.24 s]  sage t src/sage/databases/findstat.py # 80 doctests failed  Total time for all tests: 16.3 seconds cpu time: 3.0 seconds cumulative wall time: 16.2 seconds
comment:6 in reply to: ↑ 3 ; followup: ↓ 7 Changed 23 months ago by
Replying to chapoton:
This is wrong:
+ if sys.version_info[0] < 3 and isinstance(result, unicode):The "unicode" keyword cannot be used in code that is compatible with both py2 and py3. And one should refrain from using sys.version_info everywhere.
I took this from #23475:
diff git a/src/sage/structure/sage_object.pyx b/src/sage/structure/sage_object.pyx index ca1227b..6a00995 100644  a/src/sage/structure/sage_object.pyx +++ b/src/sage/structure/sage_object.pyx @@ 235,8 +235,8 @@ cdef class SageObject: return str(type(self)) else: result = repr_func()  if isinstance(result, unicode):  # Py3 compatibility: allow _repr_ to return unicode + if sys.version_info[0] < 3 and isinstance(result, unicode): + # for Py3 compatibility: allow _repr_ to return unicode return result.encode('utf8') else: return result
comment:7 in reply to: ↑ 6 ; followup: ↓ 8 Changed 23 months ago by
This is available only inside .pyx files, not for .py files
Replying to mantepse:
Replying to chapoton:
This is wrong:
+ if sys.version_info[0] < 3 and isinstance(result, unicode):The "unicode" keyword cannot be used in code that is compatible with both py2 and py3. And one should refrain from using sys.version_info everywhere.
I took this from #23475:
diff git a/src/sage/structure/sage_object.pyx b/src/sage/structure/sage_object.pyx index ca1227b..6a00995 100644  a/src/sage/structure/sage_object.pyx +++ b/src/sage/structure/sage_object.pyx @@ 235,8 +235,8 @@ cdef class SageObject: return str(type(self)) else: result = repr_func()  if isinstance(result, unicode):  # Py3 compatibility: allow _repr_ to return unicode + if sys.version_info[0] < 3 and isinstance(result, unicode): + # for Py3 compatibility: allow _repr_ to return unicode return result.encode('utf8') else: return result
comment:8 in reply to: ↑ 7 Changed 23 months ago by
Strange, because it seems to work. Anyway, I have no idea how to fix it differently, can you help?
Replying to chapoton:
This is available only inside .pyx files, not for .py files
Replying to mantepse:
Replying to chapoton:
This is wrong:
+ if sys.version_info[0] < 3 and isinstance(result, unicode):The "unicode" keyword cannot be used in code that is compatible with both py2 and py3. And one should refrain from using sys.version_info everywhere.
I took this from #23475:
diff git a/src/sage/structure/sage_object.pyx b/src/sage/structure/sage_object.pyx index ca1227b..6a00995 100644  a/src/sage/structure/sage_object.pyx +++ b/src/sage/structure/sage_object.pyx @@ 235,8 +235,8 @@ cdef class SageObject: return str(type(self)) else: result = repr_func()  if isinstance(result, unicode):  # Py3 compatibility: allow _repr_ to return unicode + if sys.version_info[0] < 3 and isinstance(result, unicode): + # for Py3 compatibility: allow _repr_ to return unicode return result.encode('utf8') else: return result
comment:9 followup: ↓ 10 Changed 23 months ago by

src/sage/databases/findstat.py
diff git a/src/sage/databases/findstat.py b/src/sage/databases/findstat.py index 5bd5fff..e6af9e0 100644
a b class FindStat(SageObject): 411 411 sage: p = findstat({pi: pi.length() for pi in l}, depth=0); p # optional  internet, random 412 412 0: (St000018: The number of inversions of a permutation., [], 873) 413 413 414 Note however, that the results of these two queries are not 415 necessarily the same, because we compare queries by the data 416 sent, and the ordering of the data might be different:: 417 418 sage: p == q # optional  internet 419 False 414 Note however, that the results of these two queries need not 415 compare equal, because we compare queries by the data 416 sent, and the ordering of the data might be different.
Not that I understand what this example is demonstrating or whether it's important, but I also don't understand why it was removed in service to Python 3 fixes? Why shouldn't this example work on Python 3? Or is it an example that is not expected to work at all (or maybe if the example is random it might be better if it were kept but marked # random
, emphasizing that the result may or may not be false).
comment:10 in reply to: ↑ 9 Changed 23 months ago by
I removed it, because with python3 it surprisingly gave True
, but more importantly, because I think it is not very interesting. When I implemented all this, I was not sure how to structure the various results returned by findstat. I now know a little better, and I have plans for a much better interface, but not really the time right now.
If you insist, I can mark the doctest as random.
Replying to embray:
src/sage/databases/findstat.py
diff git a/src/sage/databases/findstat.py b/src/sage/databases/findstat.py index 5bd5fff..e6af9e0 100644
a b class FindStat(SageObject): 411 411 sage: p = findstat({pi: pi.length() for pi in l}, depth=0); p # optional  internet, random 412 412 0: (St000018: The number of inversions of a permutation., [], 873) 413 413 414 Note however, that the results of these two queries are not 415 necessarily the same, because we compare queries by the data 416 sent, and the ordering of the data might be different:: 417 418 sage: p == q # optional  internet 419 False 414 Note however, that the results of these two queries need not 415 compare equal, because we compare queries by the data 416 sent, and the ordering of the data might be different. Not that I understand what this example is demonstrating or whether it's important, but I also don't understand why it was removed in service to Python 3 fixes? Why shouldn't this example work on Python 3? Or is it an example that is not expected to work at all (or maybe if the example is random it might be better if it were kept but marked
# random
, emphasizing that the result may or may not be false).
comment:11 Changed 23 months ago by
 Commit changed from 66931ff6189650c6b7c8a145978340dd24d627d2 to 66b263c709c0ae468916680b744fe18e843416a7
Branch pushed to git repo; I updated commit sha1. New commits:
66b263c  Merge branch 'develop' of git://trac.sagemath.org/sage into t/27346/py3__fix_findstat_interface_and_fancytuple

comment:12 Changed 23 months ago by
inside src/sage/databases/oeis.py
, use bytes_to_str
available from
from sage.cpython.string import bytes_to_str
comment:13 Changed 23 months ago by
 Commit changed from 66b263c709c0ae468916680b744fe18e843416a7 to 488e74e8ddfa36757ca0479ea540778e8338f8b9
Branch pushed to git repo; I updated commit sha1. New commits:
488e74e  remove seemingly unnecessary python3 treatment

comment:14 Changed 23 months ago by
I don't get it, now it works without any special treatment.
comment:15 Changed 23 months ago by
works for me on python3
comment:16 Changed 23 months ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok, let it be.
comment:17 Changed 23 months ago by
 Branch changed from u/mantepse/py3__fix_findstat_interface_and_fancytuple to 488e74e8ddfa36757ca0479ea540778e8338f8b9
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
make FindStat interface and FancyTuple python3 compatible