Opened 2 years ago
Closed 2 years ago
#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 2 years ago by
- Commit set to 7be6459e43ef4e9cc96d5fe990ea84335562d591
comment:2 Changed 2 years ago by
- Status changed from new to needs_review
comment:3 Changed 2 years ago by
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 2 years ago by
@sbrandhorst. If your question is how to build/run sage with python3 see Python3-compatible code
comment:5 Changed 2 years ago by
ah thank you. I will build with python3 and continue the review tomorrow.
comment:6 Changed 2 years ago by
- 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 2 years ago by
- 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 2 years ago by
- Commit changed from 7be6459e43ef4e9cc96d5fe990ea84335562d591 to 4b41a9fe9be102735ffc541afbd082227e7c5d17
comment:9 Changed 2 years ago by
- Status changed from needs_work to needs_review
Rebased and fixed the failing tests.
comment:10 Changed 2 years ago by
- 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 2 years ago by
Oh, that's silly. Thanks for checking!
comment:12 Changed 2 years ago by
- 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
comment:13 Changed 2 years ago by
Thank you!
comment:15 Changed 2 years ago by
- Branch changed from public/25694 to 986be9addc81177b269e1d87e10ef7b1dff50c43
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
py3: miscellaneous minor test fixes for Python 3