Opened 3 years ago

Closed 3 years ago

#26545 closed enhancement (fixed)

Py3: Many python3 fixes in categories

Reported by: vklein Owned by:
Priority: major Milestone: sage-8.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:

Status badges

Description


Change History (27)

comment:1 Changed 3 years ago by vklein

  • Branch set to u/vklein/develop

comment:2 Changed 3 years ago by git

  • Commit set to c0dda3fca9ddbbab3bfe93cdc8448044d755de5d

Branch pushed to git repo; I updated commit sha1. New commits:

​c0dda3fTrac #26545: Fix some issue related with ...

comment:3 Changed 3 years ago by git

  • Commit changed from c0dda3fca9ddbbab3bfe93cdc8448044d755de5d to 2f2a409f169728e26430a7d43d1d7137260fe399

Branch pushed to git repo; I updated commit sha1. New commits:

​2f2a409Trac #26545: Some python3 fix action.pyx and ...

comment:4 Changed 3 years ago by git

  • Commit changed from 2f2a409f169728e26430a7d43d1d7137260fe399 to 3771d9892416d3bad676d238bc4aeb4c4676ff62

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​3771d98Trac #26545: Python3 fixes several doctests in categories

comment:5 Changed 3 years ago by vklein

  • Keywords thursdaybdx added

comment:6 Changed 3 years ago by vklein

  • Keywords thursdaysbdx added; thursdaybdx removed

comment:7 Changed 3 years ago by git

  • Commit changed from 3771d9892416d3bad676d238bc4aeb4c4676ff62 to a5a915af70c674b5ea80a080ae55626245dafaf8

Branch pushed to git repo; I updated commit sha1. New commits:

​a5a915aTrac #26545: py3 fix doctests in categories/category.py ...

comment:8 Changed 3 years ago by git

  • Commit changed from a5a915af70c674b5ea80a080ae55626245dafaf8 to 70471d971eb4dcd46a8f5415a28c93e937917391

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​70471d9Trac #26545: py3 fix doctests in categories/category.py ...

comment:9 Changed 3 years ago by vklein

  • Status changed from new to needs_review

New commits:

​70471d9Trac #26545: py3 fix doctests in categories/category.py ...

New commits:

​70471d9Trac #26545: py3 fix doctests in categories/category.py ...

comment:10 follow-up: Changed 3 years ago by 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.

comment:11 in reply to: ↑ 10 Changed 3 years ago by vklein

  • 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 3 years ago by git

  • Commit changed from 70471d971eb4dcd46a8f5415a28c93e937917391 to b9641f2c0c5a82ec99ed79c4b9cfd904faa17b92

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​d774c9cTrac #26545: Some python3 fix in module categories
​f85a7b9Trac #26545: Fix some issue related with ...
​4386e9dTrac #26545: Python3 fixes several doctests in categories
​b9641f2Trac #26545: py3 fix doctests in categories/category.py ...

comment:13 Changed 3 years ago by vklein

  • Status changed from needs_work to needs_review

Rebased on 8.5.beta1.

comment:14 Changed 3 years ago by vklein

  • Authors set to Vincent Klein

comment:15 Changed 3 years ago by chapoton

I would suggest to undo the changes involving #py2 / #py3 and keep them for later.

comment:16 Changed 3 years ago by vklein

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 follow-up: Changed 3 years ago by chapoton

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 3 years ago by vklein

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 3 years ago by vklein

Ok it's IF PY_MAJOR_VERSION < 3: syntax instead of if six.PY2 i suppose.

comment:20 Changed 3 years ago by git

  • Commit changed from b9641f2c0c5a82ec99ed79c4b9cfd904faa17b92 to 0beeea4758aee5f4c22eea25f303b4c110f24e98

Branch pushed to git repo; I updated commit sha1. New commits:

​0beeea4Trac #26545: - remove # py2-py3 doctest syntax ...

comment:21 Changed 3 years ago by vklein

Required changes done.

comment:22 Changed 3 years ago by chapoton

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 3 years ago by git

  • Commit changed from 0beeea4758aee5f4c22eea25f303b4c110f24e98 to 07e7a7babfa8bfb6cf6e84e253c41e94672888cb

Branch pushed to git repo; I updated commit sha1. New commits:

​07e7a7bTrac #26545: py3 Replace unecessary IF/ELSE with if/else.

comment:24 Changed 3 years ago by vklein

Fixed with ​07e7a7b.

comment:25 Changed 3 years ago by git

  • Commit changed from 07e7a7babfa8bfb6cf6e84e253c41e94672888cb to f8abfa4a5d89704f1d8226a018460bd5434abf9e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​f8abfa4Trac #26545: py3 Replace unnecessary IF/ELSE with if/else

comment:26 Changed 3 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

Bien, merci.

comment:27 Changed 3 years ago by vbraun

  • Branch changed from u/vklein/develop to f8abfa4a5d89704f1d8226a018460bd5434abf9e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.