Opened 2 years ago
Closed 2 years ago
#26545 closed enhancement (fixed)
Py3: Many python3 fixes in categories
Reported by:  vklein  Owned by:  

Priority:  major  Milestone:  sage8.5 
Component:  python3  Keywords:  thursdaysbdx 
Cc:  Merged in:  
Authors:  Vincent Klein  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  f8abfa4 (Commits, GitHub, GitLab)  Commit:  f8abfa4a5d89704f1d8226a018460bd5434abf9e 
Dependencies:  Stopgaps: 
Description
Change History (27)
comment:1 Changed 2 years ago by
 Branch set to u/vklein/develop
comment:2 Changed 2 years ago by
 Commit set to c0dda3fca9ddbbab3bfe93cdc8448044d755de5d
comment:3 Changed 2 years ago by
 Commit changed from c0dda3fca9ddbbab3bfe93cdc8448044d755de5d to 2f2a409f169728e26430a7d43d1d7137260fe399
Branch pushed to git repo; I updated commit sha1. New commits:
2f2a409  Trac #26545: Some python3 fix action.pyx and ...

comment:4 Changed 2 years ago by
 Commit changed from 2f2a409f169728e26430a7d43d1d7137260fe399 to 3771d9892416d3bad676d238bc4aeb4c4676ff62
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3771d98  Trac #26545: Python3 fixes several doctests in categories

comment:5 Changed 2 years ago by
 Keywords thursdaybdx added
comment:6 Changed 2 years ago by
 Keywords thursdaysbdx added; thursdaybdx removed
comment:7 Changed 2 years ago by
 Commit changed from 3771d9892416d3bad676d238bc4aeb4c4676ff62 to a5a915af70c674b5ea80a080ae55626245dafaf8
Branch pushed to git repo; I updated commit sha1. New commits:
a5a915a  Trac #26545: py3 fix doctests in categories/category.py ...

comment:8 Changed 2 years ago by
 Commit changed from a5a915af70c674b5ea80a080ae55626245dafaf8 to 70471d971eb4dcd46a8f5415a28c93e937917391
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
70471d9  Trac #26545: py3 fix doctests in categories/category.py ...

comment:9 Changed 2 years ago by
 Status changed from new to needs_review
comment:10 followup: ↓ 11 Changed 2 years ago by
I am not convinced by the changes involving adding the tags # py2
and #py3
.
Maybe these changes should be corrected in a better way.
comment:11 in reply to: ↑ 10 Changed 2 years ago by
 Status changed from needs_review to needs_work
Replying to chapoton:
I am not convinced by the changes involving adding the tags
# py2
and#py3
.Maybe these changes should be corrected in a better way.
I am open to any suggestion.
Automerge failed => need_work.
comment:12 Changed 2 years ago by
 Commit changed from 70471d971eb4dcd46a8f5415a28c93e937917391 to b9641f2c0c5a82ec99ed79c4b9cfd904faa17b92
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d774c9c  Trac #26545: Some python3 fix in module categories

f85a7b9  Trac #26545: Fix some issue related with ...

4386e9d  Trac #26545: Python3 fixes several doctests in categories

b9641f2  Trac #26545: py3 fix doctests in categories/category.py ...

comment:13 Changed 2 years ago by
 Status changed from needs_work to needs_review
Rebased on 8.5.beta1.
comment:14 Changed 2 years ago by
comment:15 Changed 2 years ago by
I would suggest to undo the changes involving #py2 / #py3 and keep them for later.
comment:16 Changed 2 years ago by
To be accurate are you talking about all the cases with #py2/#py3
or only the ones in additive_magmas.py
and additive_semigroups.py
?
The other case in category.py is quite different by nature :
 sage: Algebras(QQ).required_methods() + sage: Algebras(QQ).required_methods() # py2 {'element': {'optional': ['_add_', '_mul_'], 'required': ['__nonzero__']}, 'parent': {'optional': ['algebra_generators'], 'required': ['__contains__']}} + sage: Algebras(QQ).required_methods() # py3 + {'element': {'optional': ['_add_', '_mul_'], 'required': ['__bool__']}, + 'parent': {'optional': ['algebra_generators'], 'required': ['__contains__']}}
the only other ways to fix that i can think of is not testing __nonzero__/__bool__
presence or modify the doctest framework ( and i think the later is not worth it for one case).
comment:17 followup: ↓ 18 Changed 2 years ago by
yes, I am just talking about undoing changes in
src/sage/categories/additive_magmas.py src/sage/categories/additive_semigroups.py
Also, in src/sage/categories/morphism.pyx, one should not import six
in pyx files.
comment:18 in reply to: ↑ 17 Changed 2 years ago by
Replying to chapoton:
... Also, in src/sage/categories/morphism.pyx, one should not
import six
in pyx files.
What should one use instead ?
Side Note : currently weak_dict.pyx
and fpickle.pyx
use import six
comment:19 Changed 2 years ago by
Ok it's IF PY_MAJOR_VERSION < 3:
syntax instead of if six.PY2
i suppose.
comment:20 Changed 2 years ago by
 Commit changed from b9641f2c0c5a82ec99ed79c4b9cfd904faa17b92 to 0beeea4758aee5f4c22eea25f303b4c110f24e98
Branch pushed to git repo; I updated commit sha1. New commits:
0beeea4  Trac #26545:  remove # py2py3 doctest syntax ...

comment:21 Changed 2 years ago by
Required changes done.
comment:22 Changed 2 years ago by
no need to capitalize if and else here:
+ IF PY_MAJOR_VERSION < 3: + return super(Morphism, self).__nonzero__() + ELSE: + return super().__bool__()
comment:23 Changed 2 years ago by
 Commit changed from 0beeea4758aee5f4c22eea25f303b4c110f24e98 to 07e7a7babfa8bfb6cf6e84e253c41e94672888cb
Branch pushed to git repo; I updated commit sha1. New commits:
07e7a7b  Trac #26545: py3 Replace unecessary IF/ELSE with if/else.

comment:24 Changed 2 years ago by
Fixed with 07e7a7b
.
comment:25 Changed 2 years ago by
 Commit changed from 07e7a7babfa8bfb6cf6e84e253c41e94672888cb to f8abfa4a5d89704f1d8226a018460bd5434abf9e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f8abfa4  Trac #26545: py3 Replace unnecessary IF/ELSE with if/else

comment:26 Changed 2 years ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
Bien, merci.
comment:27 Changed 2 years ago by
 Branch changed from u/vklein/develop to f8abfa4a5d89704f1d8226a018460bd5434abf9e
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Trac #26545: Fix some issue related with ...