Opened 2 years ago

Closed 21 months ago

#24269 closed defect (fixed)

py3: first pass at getting pexpect interfaces working (gap + maxima)

Reported by: embray Owned by:
Priority: major Milestone: sage-8.3
Component: python3 Keywords: gap maxima
Cc: Merged in:
Authors: Erik Bray Reviewers: Jeroen Demeyer, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 057668b (Commits) Commit: 057668bcb5c23ecb49c4b908b401ec7d2ae958a0
Dependencies: Stopgaps:

Description

With these changes, at the very least, I was able to get 1+1 working:

sage: gap('1+1;')
2
sage: maxima('1+1;')
2

However, see #24268 for further discussion.

Change History (39)

comment:1 Changed 2 years ago by embray

  • Dependencies set to #24222
  • Status changed from new to needs_review

comment:2 Changed 2 years ago by git

  • Commit changed from 9e76590bcadd83b8a384145445d4ac9f6f9074f0 to b893b4ea9dbccca02eb93a8fcb96305b68af8e91

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

b893b4eUpdate SagePtyProcess and the Maxima and GAP pexpect interfaces to handle bytes roughly appropriately on Python 3

comment:3 Changed 2 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work

Comment in sagespawn.pyx is no longer correct. There is also a typo targt

comment:4 Changed 2 years ago by jdemeyer

  • Dependencies #24222 deleted

comment:5 Changed 2 years ago by embray

Oh right--this commit is actually rather old at this point. I will just update the comment, rebase this, and squash.

comment:6 Changed 2 years ago by git

  • Commit changed from b893b4ea9dbccca02eb93a8fcb96305b68af8e91 to 8c5770b28d8727fc7d12cda2c0159153628e711f

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

8c5770bUpdate SagePtyProcess and the Maxima and GAP pexpect interfaces to handle bytes roughly appropriately on Python 3

comment:7 Changed 2 years ago by embray

  • Status changed from needs_work to needs_review

Fixed the comment in SagePtyProcess. Not super happy with how loosely encodings are being used in the expect interfaces currently, but at the same time the vast majority of the text going in and out of them is ASCII anyways. This is just a first pass.

comment:8 Changed 2 years ago by chapoton

Looks good to me. But I am a bit surprised by all these added b to turn strings to bytes..

comment:9 Changed 2 years ago by git

  • Commit changed from 8c5770b28d8727fc7d12cda2c0159153628e711f to afb764397db3bbe00743efbbe43daa465eafd921

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

afb7643Update SagePtyProcess and the Maxima and GAP pexpect interfaces to handle bytes roughly appropriately on Python 3

comment:10 Changed 2 years ago by jdemeyer

In principle this commit is good, but I'd to know what your overall strategy is to deal with pexpect interfaces. Right now, it seems that you just plan to add explicit conversions between bytes and str everywhere. If pexpect only deals with bytes, perhaps we should write some specific methods to encode/decode to str and use those specific methods?

comment:11 Changed 2 years ago by embray

I made #24268 for discussing overall strategy, but as you can see from this ticket and others the strategy I've gone with for now is leaving pexpect to deal in bytes, and sprinkle conversions all around. I'm not particularly happy with this strategy and fear that it might backfire.

Adding specific methods might not be a bad idea, and I've considered it. The Expect class (seemingly inexplicably) has a _before() method which just returns the .before attribute. It's used in just a handful of places, and most other interfaces use self._expect.before directly.

It might make more sense to get rid of Expect._before but add an Expect.before property that wraps self._expect.before with the relevant string conversion (and likewise for after). That might be a start in a better direction, anyways...

comment:12 Changed 2 years ago by git

  • Commit changed from afb764397db3bbe00743efbbe43daa465eafd921 to 402879bc5a2c54375509ad549d57199df8833d7a

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

0ccb5a7call bytes_to_str from Expect._before(), and add an equivalent Expect._after();
dae2109Also supply a wrapper around spawn.readline that incorporates bytes_to_str
402879bUpdate SagePtyProcess and the Maxima and GAP pexpect interfaces to handle bytes roughly appropriately on Python 3

comment:13 Changed 2 years ago by embray

  • Dependencies set to #24957

Reworked this a bit on top of #24957 which simplifies things a bit, reducing the number of manual bytes_to_str calls.

comment:14 Changed 2 years ago by git

  • Commit changed from 402879bc5a2c54375509ad549d57199df8833d7a to 8c44501f8310861a66a32cb8d555243f2519e936

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

8c44501Additional Python 3 fixes to the Maxima interface:

comment:15 Changed 2 years ago by embray

Added a commit from my big python3 branch that really should have been included here (without it the fixes in this ticket actually crashed at sage startup).

comment:16 Changed 2 years ago by chapoton

The patchbots report a doctest failure in singular expect interface.

comment:17 Changed 2 years ago by embray

There's just a missing line in the doctest I added.

comment:18 Changed 2 years ago by git

  • Commit changed from 8c44501f8310861a66a32cb8d555243f2519e936 to d59717e1d3716b51b98631279ccd577a375f1b61

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

d59717eAdd missing line to doctest

comment:19 Changed 22 months ago by embray

  • Milestone changed from sage-8.2 to sage-8.3

comment:20 Changed 21 months ago by chapoton

  • Dependencies #24957 deleted

We should really move on here now..

comment:21 Changed 21 months ago by embray

Okay? I agree... Is there anything this still needs?

comment:22 Changed 21 months ago by chapoton

For me, its ok. Though I have not tried it with python3.

Jeroen, do you have some comments ?

comment:23 Changed 21 months ago by chapoton

  • Branch changed from u/embray/python3/pexpect-gap-maxima to public/ticket/24269
  • Commit changed from d59717e1d3716b51b98631279ccd577a375f1b61 to c545cf0667bdea4db4eb6637915e6f9a4d5d78a0

rebased on 8.3.b2


New commits:

c545cf0Merge branch 'u/embray/python3/pexpect-gap-maxima' in 8.3.b2

comment:24 Changed 21 months ago by chapoton

I have checked that this does fix the broken start of sage with py3.

comment:25 Changed 21 months ago by chapoton

But not quite doing as well as promised:

sage: gap('1+1;')
2
sage: maxima('1+1;')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/expect.py in eval(self, code, strip, synchronize, locals, allow_use_file, split_lines, **kwds)
   1355                     return '\n'.join([self._eval_line(L, allow_use_file=allow_use_file, **kwds)
-> 1356                                         for L in code.split('\n') if L != ''])
   1357                 else:

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/expect.py in <listcomp>(.0)
   1355                     return '\n'.join([self._eval_line(L, allow_use_file=allow_use_file, **kwds)
-> 1356                                         for L in code.split('\n') if L != ''])
   1357                 else:

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in _eval_line(self, line, allow_use_file, wait_for_prompt, reformat, error_check, restart_if_needed)
    783 
--> 784         self._synchronize()
    785 

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in _synchronize(self)
    853                 self._expect_expr(timeout=0.5)
--> 854                 if not s in bytes_to_str(self._before()):
    855                     self._expect_expr(s,timeout=0.5)

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/cpython/string.pxd in sage.cpython.string.bytes_to_str (build/cythonized/sage/cpython/string.c:1556)()

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/cpython/string.pxd in sage.cpython.string.bytes_to_str (build/cythonized/sage/cpython/string.c:1410)()

TypeError: expected bytes, str found

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
<ipython-input-2-e25f09e1dc17> in <module>()
----> 1 maxima('1+1;')

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/interface.py in __call__(self, x, name)
    278 
    279         if isinstance(x, string_types):
--> 280             return cls(self, x, name=name)
    281         try:
    282             return self._coerce_from_special_method(x)

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in __init__(self, parent, value, is_name, name)
   1168             True
   1169         """
-> 1170         ExpectElement.__init__(self, parent, value, is_name=False, name=None)
   1171 
   1172     def display2d(self, onscreen=True):

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/expect.py in __init__(self, parent, value, is_name, name)
   1440         else:
   1441             try:
-> 1442                 self._name = parent._create(value, name=name)
   1443             # Convert ValueError and RuntimeError to TypeError for
   1444             # coercion to work properly.

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/interface.py in _create(self, value, name)
    474     def _create(self, value, name=None):
    475         name = self._next_var_name() if name is None else name
--> 476         self.set(name, value)
    477         return name
    478 

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in set(self, var, value)
   1013             self._batch(cmd, batchload=True)
   1014         else:
-> 1015             self._eval_line(cmd)
   1016             #self._sendline(cmd)
   1017             #self._expect_expr()

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in _eval_line(self, line, allow_use_file, wait_for_prompt, reformat, error_check, restart_if_needed)
    793             return repr(a)
    794         else:
--> 795             self._sendline(line)
    796 
    797         line_echo = self._readline()

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in _sendline(self, string)
    654             '9'
    655         """
--> 656         self._sendstr(string)
    657         os.write(self._expect.child_fd, os.linesep.encode('ascii'))
    658 

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/expect.py in _sendstr(self, string)
   1221         """
   1222         if self._expect is None:
-> 1223             self._start()
   1224         try:
   1225             os.write(self._expect.child_fd, str_to_bytes(string))

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/maxima.py in _start(self)
    619 
    620         """
--> 621         Expect._start(self)
    622         self._sendline(r":lisp (defun tex-derivative (x l r) (tex (if $derivabbrev (tex-dabbrev x) (tex-d x '\\partial)) l r lop rop ))")
    623 

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/expect.py in _start(self, alt_message, block_during_init)
    530             if block_during_init:
    531                 for X in self.__init_code:
--> 532                     self.eval(X)
    533             else:
    534                 for X in self.__init_code:

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/interfaces/expect.py in eval(self, code, strip, synchronize, locals, allow_use_file, split_lines, **kwds)
   1361         # In particular, do NOT call self._keyboard_interrupt()
   1362         except TypeError as s:
-> 1363             raise TypeError('error evaluating "%s":\n%s'%(code,s))
   1364 
   1365     ############################################################

TypeError: error evaluating "display2d : false":
expected bytes, str found

comment:26 Changed 21 months ago by git

  • Commit changed from c545cf0667bdea4db4eb6637915e6f9a4d5d78a0 to 9d984e22009667d7492dd48d8d7e05a4fee40e52

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

9d984e2just pyflakes fix

comment:27 Changed 21 months ago by git

  • Commit changed from 9d984e22009667d7492dd48d8d7e05a4fee40e52 to 28b894fb3faeeb3e8a8697dea6615f827697b65e

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

28b894fundo import removal

comment:28 Changed 21 months ago by embray

Well, this used to work. Let me see if I can fix it again. There is a follow-up ticket somewhere that makes several more fixes to the expect interfaces. This was just a small proof of concept change to get the bare minimum working.

comment:29 Changed 21 months ago by embray

I'm going to set this back to my branch. But I'll incorporate the fixes from pyflakes.

comment:30 Changed 21 months ago by embray

  • Branch changed from public/ticket/24269 to u/embray/python3/pexpect-gap-maxima
  • Commit changed from 28b894fb3faeeb3e8a8697dea6615f827697b65e to 25409dc6bf6c74fc5c0563305007feb8d3e4f8ea

Ok, this should be working again, just at the bare minimum level promised by the original ticket description.


New commits:

3f08a18Also supply a wrapper around spawn.readline that incorporates bytes_to_str
6c8a6f0Update SagePtyProcess and the Maxima and GAP pexpect interfaces to handle bytes roughly appropriately on Python 3
4a6a72ajust pyflakes fix
3d4bcc5undo import removal
25409dcthe base class now provides the expectation that self._prompt is already bytes

comment:31 Changed 21 months ago by chapoton

I confirm that this works on python3. Let us wait for the python2-patchbot, and then it will be good to go.

comment:32 Changed 21 months ago by embray

Huh. Weirdly a commit with the same message as 6c8a6f0 was already merged some time ago, but with different content. Oh well, the text is still basically accurate...

comment:33 Changed 21 months ago by git

  • Commit changed from 25409dc6bf6c74fc5c0563305007feb8d3e4f8ea to df96d07cad7a72216895962e9b642e3830f83fd9

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

df96d07Additional Python 3 fixes to the Maxima interface:

comment:34 Changed 21 months ago by chapoton

checked again that it works with py3; now waiting for py2 patchbots

comment:35 Changed 21 months ago by chapoton

The patchbots report a doctest failure in singular expect interface. As in comment:16 !

comment:36 Changed 21 months ago by git

  • Commit changed from df96d07cad7a72216895962e9b642e3830f83fd9 to 057668bcb5c23ecb49c4b908b401ec7d2ae958a0

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

057668bAdd missing line to doctest

comment:37 Changed 21 months ago by chapoton

checked once again that it works with py3; now waiting for py2 patchbots

Also, for the record,

sage: singular('1+1;')
2

works too.

comment:38 Changed 21 months ago by chapoton

  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, let it be

comment:39 Changed 21 months ago by vbraun

  • Branch changed from u/embray/python3/pexpect-gap-maxima to 057668bcb5c23ecb49c4b908b401ec7d2ae958a0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.