Opened 3 years ago
Closed 3 years ago
#24286 closed defect (fixed)
py3: minor fixes to sage.repl.load and sage.repl.attach
Reported by:  embray  Owned by:  

Priority:  major  Milestone:  sage8.2 
Component:  python3  Keywords:  
Cc:  jdemeyer  Merged in:  
Authors:  Erik Bray  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  ede1402 (Commits, GitHub, GitLab)  Commit:  ede1402a8ec9f4a2012584b3017d54e1c925e692 
Dependencies:  Stopgaps: 
Description
This will be one of many tickets I will be opening for miscellaneous minor fixes to specific modules.
In some cases I'll keep these more "thematic" in which case they might span multiple modules, but in other cases it is simpler to submit one module at a time for minor fixes.
See the commit messages for more details about individual fixes.
Change History (21)
comment:1 Changed 3 years ago by
 Dependencies set to #24222
comment:2 Changed 3 years ago by
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
 Cc jdemeyer added
looks good enough to me.
Jeroen, any opinion ?
EDIT: This seems to break several things, see patchbot report..
comment:4 Changed 3 years ago by
 Status changed from needs_review to needs_work
comment:5 Changed 3 years ago by
 Branch changed from u/embray/python3/sagereplload to public/python3sagereplload
 Commit changed from 5d3dce1a7e73ff7ac81a87d822f2651a6273e8f5 to 480468942951a7b55226e535e4ec8e05d045ee3d
comment:6 Changed 3 years ago by
 Branch changed from public/python3sagereplload to u/embray/python3/sagereplload
 Commit changed from 480468942951a7b55226e535e4ec8e05d045ee3d to 5d3dce1a7e73ff7ac81a87d822f2651a6273e8f5
I already had this fixed in my local branch; I just hadn't pushed it yet, so for simplicity's sake I'll keep my branch. The other issues are (mostly) related to #24475
comment:7 Changed 3 years ago by
 Summary changed from py3: minor fixes to sage.repl.load to py3: minor fixes to sage.repl.load and sage.repl.attach
comment:8 Changed 3 years ago by
 Branch changed from u/embray/python3/sagereplload to public/python3/sagereplload
 Commit changed from 5d3dce1a7e73ff7ac81a87d822f2651a6273e8f5 to 78e55f92887ca824dc802558490e14c40487607c
 Status changed from needs_work to needs_review
New commits:
78e55f9  Merge branch 'u/embray/python3/sagereplload' in 8.1.b5

comment:9 Changed 3 years ago by
 Dependencies #24222 deleted
comment:10 Changed 3 years ago by
 Commit changed from 78e55f92887ca824dc802558490e14c40487607c to 236f2a82158e430da95af81bfe52b5dadc740d53
Branch pushed to git repo; I updated commit sha1. New commits:
236f2a8  fix doctests

comment:11 Changed 3 years ago by
 Status changed from needs_review to needs_work
 In
src/sage/repl/load.py
, some doctests are marked# py2
for no good reason.
 Why use
tmp_filename
here?sage: fname = tmp_filename(ext='.py') sage: fullpath = os.path.join(t_dir, fname) sage: with open(fullpath, 'w') as f: ....: _ = f.write("print(37 * 3)")
 The git history is a mess (in particular, the commit message
Merge branch 'u/embray/python3/sagereplload' in 8.1.b5
is wrong). It would be best to squash this in one commit on top op Sage 8.2.beta5
comment:12 Changed 3 years ago by
 Commit changed from 236f2a82158e430da95af81bfe52b5dadc740d53 to be956dfdae833dd193396c8d45c2c3869ff15277
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
be956df  # Ceci est la combinaison de 2 commits.

comment:13 followup: ↓ 16 Changed 3 years ago by
I do not understand the issue in point 2
comment:14 Changed 3 years ago by
 Commit changed from be956dfdae833dd193396c8d45c2c3869ff15277 to 040cc3ff694bfc05ee9a7dff0b14025642335204
Branch pushed to git repo; I updated commit sha1. New commits:
040cc3f  trac 24286 fixing doctest

comment:15 Changed 3 years ago by
green bot
comment:16 in reply to: ↑ 13 ; followup: ↓ 20 Changed 3 years ago by
Replying to chapoton:
I do not understand the issue in point 2
The old code did not use tmp_filename
. The new code does. I see no reason for that change, so it needs to be justified.
comment:17 Changed 3 years ago by
 Commit changed from 040cc3ff694bfc05ee9a7dff0b14025642335204 to ede1402a8ec9f4a2012584b3017d54e1c925e692
Branch pushed to git repo; I updated commit sha1. New commits:
ede1402  trac 24286 detail

comment:19 Changed 3 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:20 in reply to: ↑ 16 Changed 3 years ago by
comment:21 Changed 3 years ago by
 Branch changed from public/python3/sagereplload to ede1402a8ec9f4a2012584b3017d54e1c925e692
 Resolution set to fixed
 Status changed from positive_review to closed
This will need #24222 to be merged first to make sense.