#25694 closed defect (fixed)

py3: miscellaneous test fixes in sage.structure

Reported by: embray Owned by:
Priority: minor Milestone: sage-8.4
Component: python3 Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: 986be9a (Commits) Commit: 986be9addc81177b269e1d87e10ef7b1dff50c43
Dependencies: Stopgaps:

Description

Along with a little bit of PEP-8 cleanup as well.

I don't believe the substance of any of these tests are changed (there was one case where an old-style class was updated to be a new-style class, but all that matters for the test is that it be an arbitrary object that can be weak-referenced).

Change History (15)

comment:1 Changed 16 months ago by git

  • Commit set to 7be6459e43ef4e9cc96d5fe990ea84335562d591

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

7be6459py3: miscellaneous minor test fixes for Python 3

comment:2 Changed 16 months ago by embray

  • Status changed from new to needs_review

comment:3 Changed 16 months ago by sbrandhorst

Dear Erik, I can review this ticket. But it is my first py3 review. What do I need to do? Best, Simon

comment:4 Changed 16 months ago by vklein

@sbrandhorst. If your question is how to build/run sage with python3 see Python3-compatible code

comment:5 Changed 16 months ago by sbrandhorst

ah thank you. I will build with python3 and continue the review tomorrow.

comment:6 Changed 16 months ago by embray

  • Status changed from needs_review to needs_work

It looks like there were a few test failures. They're trivial but should be fixed.

comment:7 Changed 15 months ago by embray

  • Milestone changed from sage-8.3 to sage-8.4

I believe this issue can reasonably be addressed for Sage 8.4.

comment:8 Changed 14 months ago by git

  • Commit changed from 7be6459e43ef4e9cc96d5fe990ea84335562d591 to 4b41a9fe9be102735ffc541afbd082227e7c5d17

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

04af88apy3: miscellaneous minor test fixes for Python 3
4b41a9fThese should be spelled '<type ...>' for Python 2 compatibility; the Python 3 spelling is handled automatically by the test framework

comment:9 Changed 14 months ago by embray

  • Status changed from needs_work to needs_review

Rebased and fixed the failing tests.

comment:10 Changed 14 months ago by vklein

  • Reviewers set to Vincent Klein
  • Status changed from needs_review to needs_work

Almost a positive review, the very last modification i checked failed in py3 :

File "src/sage/structure/unique_representation.py", line 1244, in sage.structure.unique_representation.UniqueRepresentation
Failed example:
    isinstance(GF(7), GF)  # py3
Expected:
    Traceback (most recent call last):
    ...
    TypeError: isinstance() arg 2 must be a class, type, or tuple of
    classes and types
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 650, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1061, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.structure.unique_representation.UniqueRepresentation[7]>", line 1, in <module>
        isinstance(GF(Integer(7)), GF)  # py3
    TypeError: isinstance() arg 2 must be a type or tuple of types

Note that your #py2 and #py3 cases expected the same results right now :

+        sage: isinstance(GF(7), GF)  # py2
         Traceback (most recent call last):
         ...
         TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types
+        sage: isinstance(GF(7), GF)  # py3
+        Traceback (most recent call last):
+        ...
+        TypeError: isinstance() arg 2 must be a class, type, or tuple of
+        classes and types

comment:11 Changed 14 months ago by embray

Oh, that's silly. Thanks for checking!

comment:12 Changed 13 months ago by chapoton

  • Branch changed from u/embray/python3/sage-structure/misc-test-fixes to public/25694
  • Commit changed from 4b41a9fe9be102735ffc541afbd082227e7c5d17 to 986be9addc81177b269e1d87e10ef7b1dff50c43
  • Status changed from needs_work to needs_review

I made the correction


New commits:

17afe48Merge branch 'u/embray/python3/sage-structure/misc-test-fixes' in 8.4.b5
986be9atrac 25694 fixing py3 doctest

comment:13 Changed 13 months ago by embray

Thank you!

comment:14 Changed 13 months ago by vklein

  • Status changed from needs_review to positive_review

Ok then.

comment:15 Changed 13 months ago by vbraun

  • Branch changed from public/25694 to 986be9addc81177b269e1d87e10ef7b1dff50c43
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.