Opened 5 years ago
Closed 5 years ago
#20814 closed enhancement (fixed)
py3 print : not tested cases, step 5
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.3 |
Component: | python3 | Keywords: | |
Cc: | tscrim | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Jori Mäntysalo, Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | fc51d8e (Commits, GitHub, GitLab) | Commit: | fc51d8e01998efca5901063886723ff0437f2bd7 |
Dependencies: | Stopgaps: |
Description (last modified by )
another small step towards python3
some more print converted to use python3 syntax
Change History (27)
comment:1 Changed 5 years ago by
- Branch set to public/20814
- Commit set to fd1af37f1c6f4d520f86b6ae86ccc57234380180
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
- Reviewers set to Jori Mäntysalo
- Status changed from needs_review to needs_work
An error here:
- if verbose: print "Check:", + if verbose: + print("Check:", end="")
needs a space after colon.
comment:3 Changed 5 years ago by
comment:4 Changed 5 years ago by
- Commit changed from fd1af37f1c6f4d520f86b6ae86ccc57234380180 to 93e0bb5e166e2c2fa97a1744cedf54af46ba74e4
Branch pushed to git repo; I updated commit sha1. New commits:
93e0bb5 | trac 20814 detail
|
comment:5 Changed 5 years ago by
- Status changed from needs_work to positive_review
comment:6 Changed 5 years ago by
- Status changed from positive_review to needs_work
Merge conflict, wait for next beta
comment:7 Changed 5 years ago by
- Commit changed from 93e0bb5e166e2c2fa97a1744cedf54af46ba74e4 to 08967ef6b3631a025884b89261ce517d056c1403
Branch pushed to git repo; I updated commit sha1. New commits:
08967ef | Merge branch 'public/20814' into 7.3.b4
|
comment:8 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:9 Changed 5 years ago by
- Status changed from needs_review to needs_work
comment:10 Changed 5 years ago by
- Commit changed from 08967ef6b3631a025884b89261ce517d056c1403 to a7146ea37f83bf67e1c2be6294018bf6586f359f
Branch pushed to git repo; I updated commit sha1. New commits:
a7146ea | correct or disactivate the last python2-only print doctests
|
comment:11 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:13 Changed 5 years ago by
bot is green
comment:14 follow-up: ↓ 15 Changed 5 years ago by
So with this, every time from this point forward we can never have a Python2 print statement in the doctests? Do I understand the ticket description correctly?
comment:15 in reply to: ↑ 14 Changed 5 years ago by
Replying to tscrim:
So with this, every time from this point forward we can never have a Python2 print statement in the doctests? Do I understand the ticket description correctly?
Yes, I think so. this seems to be triggered by the change in structure/test_factory.py.
I was not expecting that, but as this was something I wanted to do anyway, I am happy
to do it here.
EDIT: I do not understand by what change this is triggered. Maybe one in misc ?
1) I have not tested the new behaviour by adding a fake file with a bad print 2) one should also check that the print behaviour in console and notebooks is still py2 3) maybe this should be discussed on sage-devel, though this is not a major change really 4) maybe this can/should be split in another ticket ?
This is the keystone of all my previous changes in the doc, that prevents any regression.
comment:16 Changed 5 years ago by
I have tested by adding a fake new file with a bad print in the doc: tests do not pass
I have checked that the behaviour of print is unchanged in console and both notebooks.
I still do not understand why this desirable behaviour happens.
comment:17 Changed 5 years ago by
ping ?
comment:18 Changed 5 years ago by
I think it was the change in misc/dev_tools.py
. While in principle, I am for this change, I do believe we should have some discussion on sage-devel about it. In general, we can write 2/3 compatible docstrings, but we cannot strictly enforce it. I am in favor of enforcing the Python3 syntax, but it does create a discrepancy (at least if we add a strictly Python3 print statement). If there are no objections on sage-devel, then I am content with putting this to a positive review.
comment:19 Changed 5 years ago by
I do not think that it comes from the changes in misc/dev_tools.py
I have really tried to see where the change of behaviour came from, with no success so far.
I would now prefer this ticket not to make this change of behaviour, but for this I need to understand what has to be undone, and I have not clue.
comment:20 Changed 5 years ago by
- Commit changed from a7146ea37f83bf67e1c2be6294018bf6586f359f to d832810a08c0978cabaefd2bd86c310bb79ac993
Branch pushed to git repo; I updated commit sha1. New commits:
d832810 | trac 20814 restore the exact file where future import is not yet welcome
|
comment:21 Changed 5 years ago by
- Description modified (diff)
ok, I think I have found that the problems were coming from the future import in src/sage/modular/etaproducts.py
No idea what happens exactly, but undoing this change gives us back the usual behaviour.
So this is back to being a very simple print ticket, one of the last ones for py files.
comment:22 Changed 5 years ago by
- Reviewers changed from Jori Mäntysalo to Jori Mäntysalo, Travis Scrimshaw
If you could remove the added space in the copyright header for consistency, then you can set a positive review on my behalf.
comment:23 Changed 5 years ago by
The added space remove I can (Yoda mode), but this change is suggested by pep8. Should I remove ?
comment:24 Changed 5 years ago by
I think so. We consistently break the PEP8 in this case, and in our doc about file headers, the copyright is given without a space as well.
comment:25 Changed 5 years ago by
- Commit changed from d832810a08c0978cabaefd2bd86c310bb79ac993 to fc51d8e01998efca5901063886723ff0437f2bd7
Branch pushed to git repo; I updated commit sha1. New commits:
fc51d8e | tarc 20814 not pep8 for copyright header
|
comment:27 Changed 5 years ago by
- Branch changed from public/20814 to fc51d8e01998efca5901063886723ff0437f2bd7
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
more print converted to python3 in py files
more print converted to py3 in .py files