Opened 10 years ago

Closed 5 years ago

#14153 closed defect (fixed)

Add Unicode support to the doctesting framework

Reported by: Jeroen Demeyer Owned by: David Roe
Priority: major Milestone: sage-8.1
Component: doctest framework Keywords: python3, unicode
Cc: Leif Leonhardy, Erik Bray, Jeroen Demeyer, Florent Hivert Merged in:
Authors: Frédéric Chapoton Reviewers: Erik Bray, Volker Braun, David Roe
Report Upstream: N/A Work issues:
Branch: ed1e3b5 (Commits, GitHub, GitLab) Commit: ed1e3b5bf17e6cc923e28b11862496dd0e1b9b04
Dependencies: Stopgaps:

Status badges

Description (last modified by Frédéric Chapoton)

Support

  1. Doctest (i.e. sage:) lines containing non-ASCII characters.
  2. Doctest results containing non-ASCII characters.
  3. Print statements in doctests outputting unicode.
  4. Source files with a PEP 0263 encoding declaration.

see also #18370

Attachments (1)

doctest_unicode.patch (6.9 KB) - added by Jeroen Demeyer 10 years ago.

Download all attachments as: .zip

Change History (79)

comment:1 Changed 10 years ago by Jeroen Demeyer

Description: modified (diff)

Changed 10 years ago by Jeroen Demeyer

Attachment: doctest_unicode.patch added

comment:2 Changed 10 years ago by Leif Leonhardy

Cc: Leif Leonhardy added

comment:3 Changed 10 years ago by François Bissey

I just tried this patch on the suspicion it could help with the test weirdness I reported in 5.9.beta0 on sage-release

sage -t --long devel/sage-main/sage/misc/interpreter.py
**********************************************************************
File "devel/sage-main/sage/misc/interpreter.py", line 150, in 
sage.misc.interpreter.sage_prompt
Failed example:
    shell.run_cell('sage_prompt()')
Expected:
    u'sage'
Got:
    u'sage'
**********************************************************************
File "devel/sage-main/sage/misc/interpreter.py", line 187, in 
sage.misc.interpreter.SageInteractiveShell.system_raw
Failed example:
    shell.system_raw('R --version')
Expected:
    R version ...
Got:
    WARNING: ignoring environment value of R_HOME
    R version 2.15.2 (2012-10-26) -- "Trick or Treat"
    Copyright (C) 2012 The R Foundation for Statistical Computing
    ISBN 3-900051-07-0
    Platform: x86_64-unknown-linux-gnu (64-bit)
    <BLANKLINE>
    R is free software and comes with ABSOLUTELY NO WARRANTY.
    You are welcome to redistribute it under the terms of the
    GNU General Public License versions 2 or 3.
    For more information about these matters see
    http://www.gnu.org/licenses/.
    <BLANKLINE>
**********************************************************************
File "devel/sage-main/sage/misc/interpreter.py", line 566, in 
sage.misc.interpreter.interface_shell_embed
Failed example:
    shell.run_cell('List( [1..10], IsPrime )')
Expected:
    [ false, true, true, false, true, false, true, false, false, false ]
Got:
    [ false, true, true, false, true, false, true, false, false, false ]
**********************************************************************
3 items had failures:
   1 of  15 in sage.misc.interpreter.SageInteractiveShell.system_raw
   1 of   4 in sage.misc.interpreter.interface_shell_embed
   1 of   4 in sage.misc.interpreter.sage_prompt
    [107 tests, 3 failures, 2.4 s]

The R error was fixed by unsetting R_HOME before running the test. Then I applied the patch and here is the result

sage -t --long devel/sage-main/sage/misc/interpreter.py
**********************************************************************
File "devel/sage-main/sage/misc/interpreter.py", line 150, in sage.misc.interpreter.sage_prompt
Failed example:
    shell.run_cell('sage_prompt()')
Expected:
    u'sage'
Got:
    u'sage'
    ---------------------------------------------------------------------------
    AttributeError                            Traceback (most recent call last)
    <ipython-input-1-e17c2c2153ac> in <module>()
    ----> 1 sage_prompt()
    
    /home/work/fbissey/sandbox/sage-5.9.beta0/local/lib/python2.7/site-packages/IPython/core/displayhook.pyc in __call__(self, result)
        240             self.update_user_ns(result)
        241             self.log_output(format_dict)
    --> 242             self.finish_displayhook()
        243 
        244     def flush(self):
    <BLANKLINE>
    /home/work/fbissey/sandbox/sage-5.9.beta0/local/lib/python2.7/site-packages/IPython/core/displayhook.pyc in finish_displayhook(self)
        224         """Finish up all displayhook activities."""
        225         io.stdout.write(self.shell.separate_out2)
    --> 226         io.stdout.flush()
        227 
        228     def __call__(self, result=None):
    <BLANKLINE>
    AttributeError: IOStream instance has no attribute 'flush'
**********************************************************************
File "devel/sage-main/sage/misc/interpreter.py", line 566, in sage.misc.interpreter.interface_shell_embed
Failed example:
    shell.run_cell('List( [1..10], IsPrime )')
Expected:
    [ false, true, true, false, true, false, true, false, false, false ]
Got:
    [ false, true, true, false, true, false, true, false, false, false ]
    ---------------------------------------------------------------------------
    AttributeError                            Traceback (most recent call last)
    <ipython-input-1-a772973ec1ce> in <module>()
    ----> 1 sage.misc.all.logstr("""[ false, true, true, false, true, false, true, false, false, false ]""")
    
    /home/work/fbissey/sandbox/sage-5.9.beta0/local/lib/python2.7/site-packages/IPython/core/displayhook.pyc in __call__(self, result)
        240             self.update_user_ns(result)
        241             self.log_output(format_dict)
    --> 242             self.finish_displayhook()
        243 
        244     def flush(self):
    <BLANKLINE>
    /home/work/fbissey/sandbox/sage-5.9.beta0/local/lib/python2.7/site-packages/IPython/core/displayhook.pyc in finish_displayhook(self)
        224         """Finish up all displayhook activities."""
        225         io.stdout.write(self.shell.separate_out2)
    --> 226         io.stdout.flush()
        227 
        228     def __call__(self, result=None):
    <BLANKLINE>
    AttributeError: IOStream instance has no attribute 'flush'
**********************************************************************
2 items had failures:
   1 of   4 in sage.misc.interpreter.interface_shell_embed
   1 of   4 in sage.misc.interpreter.sage_prompt
    [107 tests, 2 failures, 3.0 s]

comment:4 Changed 10 years ago by David Roe

Component: doctestdoctest framework
Owner: changed from Minh Van Nguyen to David Roe

comment:5 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:6 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:7 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:8 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:9 Changed 6 years ago by Jeroen Demeyer

Keywords: python3 added

comment:10 Changed 6 years ago by Leif Leonhardy

Milestone: sage-6.4sage-7.4

comment:11 Changed 6 years ago by Frédéric Chapoton

Branch: public/14153
Commit: cffdb9077dc8074a0cde0d0cd21d19046b2a95d5

just made a branch from the old patch, not tried to test it


New commits:

cffdb90trac 14153 unicode in documentation

comment:12 Changed 6 years ago by git

Commit: cffdb9077dc8074a0cde0d0cd21d19046b2a95d51712c87b8afdb2543e593b6e44165aefc52fdef5

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

abd83e5Merge branch 'public/14153' in 7.5.1
1712c87trac 14153 fixing doc default call

comment:13 Changed 6 years ago by Frédéric Chapoton

I have repaired a detail, but this is not working, in several distinct ways.

This seems to be broken on lines having unicode characters in comments such as:

sage: 0 + 0    # c'est trop bête

Some doctests come from the notebook, I made a pull request.

A failing doctest in src/sage/misc/rest_index_of_methods.py is related to the presence of "Lov\xc3\xa1sz theta"

Last edited 6 years ago by Frédéric Chapoton (previous) (diff)

comment:14 Changed 6 years ago by Frédéric Chapoton

seems that something like that is hurting us (in forker):

sage: md=hashlib.md5()
sage: md.update(u'été')
---------------------------------------------------------------------------
UnicodeEncodeError                        Traceback (most recent call last)
<ipython-input-9-6a9959b36b47> in <module>()
----> 1 md.update(u'été')

UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 0: ordinal not in range(128)

solution: encode ?

sage: md.update(u'été'.encode('utf8'))
Last edited 6 years ago by Frédéric Chapoton (previous) (diff)

comment:15 Changed 6 years ago by Frédéric Chapoton

  • another problem in src/sage/misc/cachefunc.pyx with a mu:
    398:    625 loops, best of 3: 1.3 µs per loop
    

and same in src/sage/combinat/designs/evenly_distributed_sets.pyx and in src/sage/structure/list_clone_timings.py

  • in src/sage_setup/autogen/pari/doc.py, some unicode strings with no u:
    113:    doc = doc.replace("@[pm]", "±")
    115:    doc = doc.replace("@[agrave]", "à")
    116:    doc = doc.replace("@[aacute]", "á")
    117:    doc = doc.replace("@[eacute]", "é")
    118:    doc = doc.replace("@[ouml]", "ö")
    119:    doc = doc.replace("@[uuml]", "ü")
    120:    doc = doc.replace("\\'{a}", "á")
    
Last edited 6 years ago by Frédéric Chapoton (previous) (diff)

comment:16 Changed 6 years ago by git

Commit: 1712c87b8afdb2543e593b6e44165aefc52fdef58cd268138984060c14b6b43e9ab2604317a9b20a

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

8cd2681trac 14153 adding uft8 declaration in 2 files

comment:17 Changed 6 years ago by Frédéric Chapoton

Milestone: sage-7.4sage-8.0

This should be taken care of.

comment:18 Changed 6 years ago by Frédéric Chapoton

Cc: Erik Bray added

comment:19 Changed 6 years ago by git

Commit: 8cd268138984060c14b6b43e9ab2604317a9b20a489d13f0cd3640a28c05b42077b307173f0c495a

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

489d13fMerge branch 'public/14153' in 8.0.b4

comment:20 Changed 6 years ago by Frédéric Chapoton

Description: modified (diff)

comment:21 Changed 6 years ago by Frédéric Chapoton

Keywords: unicode added

comment:22 Changed 6 years ago by Erik Bray

Reviewers: Erik Bray

Looks fine to me, though I did not test it. I'm amazed there isn't anything in the stdlib to check the PEP 263 coding line, but if there is I couldn't find it.

This will be nice to have. It reminds me of my thought the other day to propose adding a few unicode constants (and the ability to tab-complete them easily) to Sage (e.g. for pi). Has that been proposed before?

comment:23 Changed 6 years ago by Frédéric Chapoton

I have not tested either, so far. May launch my patchbot on the branch later.

comment:24 Changed 6 years ago by Frédéric Chapoton

The patchbot is not happy at all !

One example of failure:

File "src/doc/ja/a_tour_of_sage/index.rst", line 39, in doc.ja.a_tour_of_sage.index
Failed example:
    x = var('x')   # 記号変数を定義
Exception raised:
    Traceback (most recent call last):
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 520, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 896, in compile_and_execute
        self.update_digests(example)
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 783, in update_digests
        self.running_global_digest.update(s)
    UnicodeEncodeError: 'ascii' codec can't encode characters in position 20-26: ordinal not in range(128)

Another kind of example:

File "src/sage/ext/fast_callable.pyx", line 34, in sage.ext.fast_callable
Failed example:
    timeit('wilk.subs(x=30)') # random, long time
Exception raised:
    Traceback (most recent call last):
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 520, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 883, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.ext.fast_callable[5]>", line 1, in <module>
        timeit('wilk.subs(x=30)') # random, long time
      File "sage/misc/sage_timeit_class.pyx", line 118, in sage.misc.sage_timeit_class.SageTimeit.__call__ (build/cythonized/sage/misc/sage_timeit_class.c:1411)
        print(self.eval(code, globals, preparse=preparse, **kwds))
      File "/home/chapoton/sage/local/lib/python2.7/codecs.py", line 369, in write
        data, consumed = self.encode(object, self.errors)
    UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 26: ordinal not in range(128)

comment:25 Changed 6 years ago by Erik Bray

To be clear, is that a problem with the patchbot or the doctest runner itself? It seems more like the latter, but the question is does this happen when running the tests outside the patchbot?

comment:26 Changed 6 years ago by François Bissey

Somewhat reminiscent of https://trac.sagemath.org/ticket/22756#comment:14 although the japanese stuff is definitely unicode and should be recognized as such by python.

comment:27 Changed 6 years ago by Frédéric Chapoton

just fails in the same way outside of the patchbot

comment:28 Changed 6 years ago by git

Commit: 489d13f0cd3640a28c05b42077b307173f0c495ab9295dca7f1e4c7caa4f6173457a7560f500de40

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

b9295dctrac 14153 encoding required

comment:29 Changed 6 years ago by git

Commit: b9295dca7f1e4c7caa4f6173457a7560f500de40be11f06e98fc93888967ae991f4ea91ce33c50e0

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

be11f06trac 14153 fixing some doctests

comment:30 Changed 6 years ago by Frédéric Chapoton

Cc: Jeroen Demeyer Florent Hivert added

Less failing doctests, but still many. Is there an expert that could help ?

comment:31 Changed 6 years ago by Erik Bray

Depends on exactly what expertise is needed, but I'll have a look.

comment:32 in reply to:  29 Changed 6 years ago by Erik Bray

Replying to git:

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

be11f06trac 14153 fixing some doctests

Regarding this commit, and others like it--is this not going to be a problem on Python 3, where the strings won't be repr'd as u'...'?

In other projects I've modified the doctest checker to ignore the u marker in strings by default. If a test needs to explicitly check whether or not a string is unicode there are other ways. But most of the time that isn't relevant.

comment:33 Changed 5 years ago by git

Commit: be11f06e98fc93888967ae991f4ea91ce33c50e00c611c5ad8aad31d5403155c40d87f2fb9e087ef

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

0c611c5trac 14153 unicode in documentation

comment:34 Changed 5 years ago by Frédéric Chapoton

Branch: public/14153u/chapoton/14153
Commit: 0c611c5ad8aad31d5403155c40d87f2fb9e087ef080916e00f751500b75fba8c992a7c699ca61ed2

still very broken..


New commits:

10bc1abtrac 14153 unicode in documentation
080916epy3: poor man's try towards unicode doctest framework

comment:35 Changed 5 years ago by git

Commit: 080916e00f751500b75fba8c992a7c699ca61ed2f3d375ed3aaa3b2c72c9fa2b30b89ac78d0d85f8

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

cf07743some unicode changes in timeit
f3d375eMerge branch "unicode in timeit" into "unicode in doctest"

comment:36 Changed 5 years ago by git

Commit: f3d375ed3aaa3b2c72c9fa2b30b89ac78d0d85f87906a504011eb32f7890c0750d3d987568576655

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

7906a50trac 14153 unicode in documentation

comment:37 Changed 5 years ago by git

Commit: 7906a504011eb32f7890c0750d3d9875685766551813a63c6e37170300f76df3a8e818b6289b6a63

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

1813a63trac 14153 some details

comment:38 Changed 5 years ago by Frédéric Chapoton

Branch: u/chapoton/14153u/chapoton/14153v2
Commit: 1813a63c6e37170300f76df3a8e818b6289b6a63e3497e851f155914d54174ad18bd28123f6b37a0

here is another tentative


New commits:

e3497e8steps towards unicode doctesting framework

comment:39 Changed 5 years ago by git

Commit: e3497e851f155914d54174ad18bd28123f6b37a0cc03d22738a327999db7162c095daac7d23f9c31

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

cc03d22motorhead detail

comment:40 Changed 5 years ago by git

Commit: cc03d22738a327999db7162c095daac7d23f9c31b091e6a60311ab5756d2fb744e60adabae120a5a

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

b091e6atrac 14153 fixing another doctest

comment:41 Changed 5 years ago by Frédéric Chapoton

There remains only one failing doctest in the r interface.

comment:42 Changed 5 years ago by git

Commit: b091e6a60311ab5756d2fb744e60adabae120a5a8394952acfe919aba99695f84248edbc212a3c9c

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

8394952trac 14153 fixing another doctest

comment:43 Changed 5 years ago by Frédéric Chapoton

Green Bot ! Please review !

I am not sure that this is the definitive fix, but at least good progress.

This needs to be tested also on Mac and Cygwin, and this I cannot do.

comment:44 Changed 5 years ago by Frédéric Chapoton

Authors: Frédéric Chapoton

comment:45 Changed 5 years ago by Frédéric Chapoton

Milestone: sage-8.0sage-8.1
Priority: minormajor
Status: newneeds_review

comment:46 Changed 5 years ago by Frédéric Chapoton

ping, anybody ?

comment:47 Changed 5 years ago by François Bissey

Testing on OS X. But it overall looks good.

comment:48 Changed 5 years ago by Erik Bray

I don't think there's anything about this that specifically needs testing on Cygwin. I've never encountered any issues on Cygwin particularly pertaining to unicode in Python.

comment:49 Changed 5 years ago by Frédéric Chapoton

ok, well. Then please review.

comment:50 Changed 5 years ago by Erik Bray

This is maybe a minor point, but here:

  • src/sage/doctest/forker.py

    diff --git a/src/sage/doctest/forker.py b/src/sage/doctest/forker.py
    index 9a5d982..705b718 100644
    a b class SageDocTestRunner(doctest.DocTestRunner): 
    514514            finally:
    515515                if self.debugger is not None:
    516516                    self.debugger.set_continue() # ==== Example Finished ====
    517             got = self._fakeout.getvalue()  # the actual output
     517            got = self._fakeout.getvalue().decode('utf-8')
     518            # the actual output
     519

it might be good if this were wrapped in a try/except, and fall back on latin1 if utf-8 decoding fails, like:

got = self._fakeout.getvalue()
try:
    got = got.decode('utf-8')
except UnicodeDecodeError:
    got = got.decode('latin1')

Of course this shouldn't happen, but in the off chance some test produces garbage output, this will at least prevent the doctest runner from crashing with a UnicodeDecodeError. Instead you'll get some text, likely wrong, that can still be compared to the expected result.

comment:51 Changed 5 years ago by Erik Bray

Likewise here:

  • src/sage/interfaces/r.py

    diff --git a/src/sage/interfaces/r.py b/src/sage/interfaces/r.py
    index 9db603f..e9096df 100644
    a b class R(ExtraTabCompletion, Expect): 
    668668            ...
    669669            ImportError: ...
    670670        """
    671         ret = self.eval('require("%s")'%library_name)
     671        ret = self.eval('require("%s")'%library_name).decode('utf-8')
    672672        # try hard to parse the message string in a locale-independent way
    673673        if ' library(' in ret:       # locale-independent key-word
    674674            raise ImportError("%s"%ret)

In this case I'm not really sure what the best thing is to do if there's an encoding error, but it should probably be handled. Will have to give that more thought. Basically any time you're decoding bytes coming from an external source it's important not to assume they'll always have the expected encoding :(

comment:52 Changed 5 years ago by Erik Bray

The rest makes sense to me. I haven't done a thorough investigation into all the places where Sage needs better unicode handling, and I'm sure this isn't exhaustive either, but I know you've done a lot of testing. The next issue is just what the patchbots will say....

comment:53 Changed 5 years ago by Frédéric Chapoton

patchbots said green already..

EDIT oh, no.. I missed the last report.

Last edited 5 years ago by Frédéric Chapoton (previous) (diff)

comment:54 Changed 5 years ago by git

Commit: 8394952acfe919aba99695f84248edbc212a3c9c4dc084e452d83fe8bf80b105d25b404a21092be7

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

0cffa8eMerge branch 'u/chapoton/14153v2' of ssh://trac.sagemath.org:22/sage into unicode
4dc084etrac 14153 reviewer's suggestions + gap doc interface

comment:55 Changed 5 years ago by Frédéric Chapoton

Status: needs_reviewneeds_work

Damn, some of the patchbots see some problems with the gap interface, that I do not see locally..

comment:56 Changed 5 years ago by François Bissey

All clear on OS X as far as I can tell.

comment:57 in reply to:  55 Changed 5 years ago by Erik Bray

Replying to chapoton:

Damn, some of the patchbots see some problems with the gap interface, that I do not see locally..

It probably has something to do with their locale settings. This is why I suggested not just assuming .decode('utf-8') will work :) Now the question is, how exactly does GAP determine what encoding to use...

comment:58 Changed 5 years ago by Erik Bray

Ah, I forgot to point that out here:

  • src/sage/interfaces/gap.py

    diff --git a/src/sage/interfaces/gap.py b/src/sage/interfaces/gap.py
    index dd7b27f..7e765a1 100644
    a b from sage.interfaces.tab_completion import ExtraTabCompletion 
    188188from sage.structure.element import ModuleElement
    189189import re
    190190import os
     191import io
    191192import pexpect
    192193import time
    193194import platform
    class Gap(Gap_generic): 
    13391340            (sline,) = match.groups()
    13401341            if self.is_remote():
    13411342                self._get_tmpfile()
    1342             F = open(self._local_tmpfile(),"r")
     1343            F = io.open(self._local_tmpfile(), "r", encoding='utf-8')
    13431344            help = F.read()
    13441345            if pager:
    13451346                from IPython.core.page import page

Let's see if we can determine what encoding GAP is actually using...

comment:59 Changed 5 years ago by Frédéric Chapoton

Maybe we should force the value of "GAPInfo.TermEncoding" ?

see https://www.gap-system.org/Manuals/pkg/GAPDoc-1.5.1/doc/chap5.html

Maybe using GAPInfo.TermEncoding := "UTF-8";

I know nothing of GAP, some input from a GAP expert could be useful here..

Last edited 5 years ago by Frédéric Chapoton (previous) (diff)

comment:60 Changed 5 years ago by Frédéric Chapoton

I can reproduce the failing doctests locally by adding one line

         self.eval('SetGAPDocTextTheme("none")')
+        self.eval('GAPInfo.TermEncoding := "latin1";')
         self.eval(r'\$SAGE.tempfile := "%s";'%tmp_to_use)

comment:61 Changed 5 years ago by git

Commit: 4dc084e452d83fe8bf80b105d25b404a21092be71c0dd43920d5c81b164992fbaf9ecd9f81a3154d

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

1c0dd43trac 14153 trying to fix the gap doc doctests

comment:62 Changed 5 years ago by Frédéric Chapoton

not sure at all that this is a good solution..

comment:63 Changed 5 years ago by Erik Bray

Interesting. It's not clear to me where TermEncoding is used in the process of displaying some help docs. But if you were able to reproduce by setting that then it must be involved somewhere, somehow...

comment:64 Changed 5 years ago by Frédéric Chapoton

see the section 5.3-2 GAPDoc2Text in the link ​https://www.gap-system.org/Manuals/pkg/GAPDoc-1.5.1/doc/chap5.html

I think this explain what GAP does when its doc is required as a text.

comment:65 Changed 5 years ago by Erik Bray

Great, that should explain it then.

Hmm--maybe rather than forcing TermEncoding to a specific value we should read TermEncoding and use that to determine how to decode text from GAP.

comment:66 Changed 5 years ago by git

Commit: 1c0dd43920d5c81b164992fbaf9ecd9f81a3154d0faa9436561f00cf1ed5b9c8a65ca4667032f2dd

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

0faa943trac 14153 detect gap encoding

comment:67 Changed 5 years ago by Frédéric Chapoton

indeed, here is a try..

comment:68 Changed 5 years ago by Frédéric Chapoton

Status: needs_workneeds_review

comment:69 Changed 5 years ago by Frédéric Chapoton

bot seems to be stable-green, please review

comment:70 Changed 5 years ago by Frédéric Chapoton

ping ? Jeroen ? Erik ?

comment:71 Changed 5 years ago by Volker Braun

Can you replace the unconditional except: with something more sensible? Probably just name = str(name) instead of the entire try/except + assertion. Rest lgtm

comment:72 Changed 5 years ago by git

Commit: 0faa9436561f00cf1ed5b9c8a65ca4667032f2dded1e3b5bf17e6cc923e28b11862496dd0e1b9b04

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

b6077c0Merge branch 'u/chapoton/14153v2' in 8.1.b1
ed1e3b5trac 14153 more precise except clause

comment:73 Changed 5 years ago by Frédéric Chapoton

Thanks Volker for having a look.

I have made a more precise except. I guess one could also have removed the try-except.

comment:74 Changed 5 years ago by Frédéric Chapoton

bot is morally green

comment:75 Changed 5 years ago by David Roe

Reviewers: Erik BrayErik Bray, Volker Bruan, David Roe
Status: needs_reviewpositive_review

Looks good. Sorry that this review took so long!

comment:76 Changed 5 years ago by Volker Braun

Reviewers: Erik Bray, Volker Bruan, David RoeErik Bray, Volker Braun, David Roe

comment:77 Changed 5 years ago by David Roe

Sorry for the typo Volker!

comment:78 Changed 5 years ago by Volker Braun

Branch: u/chapoton/14153v2ed1e3b5bf17e6cc923e28b11862496dd0e1b9b04
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.