Opened 6 years ago

Closed 2 years ago

#14153 closed defect (fixed)

Add Unicode support to the doctesting framework

Reported by: jdemeyer Owned by: roed
Priority: major Milestone: sage-8.1
Component: doctest framework Keywords: python3, unicode
Cc: leif, embray, jdemeyer, hivert Merged in:
Authors: Frédéric Chapoton Reviewers: Erik Bray, Volker Braun, David Roe
Report Upstream: N/A Work issues:
Branch: ed1e3b5 (Commits) Commit: ed1e3b5bf17e6cc923e28b11862496dd0e1b9b04
Dependencies: Stopgaps:

Description (last modified by 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 jdemeyer 6 years ago.

Download all attachments as: .zip

Change History (79)

comment:1 Changed 6 years ago by jdemeyer

  • Description modified (diff)

Changed 6 years ago by jdemeyer

comment:2 Changed 6 years ago by leif

  • Cc leif added

comment:3 Changed 6 years ago by fbissey

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 6 years ago by roed

  • Component changed from doctest to doctest framework
  • Owner changed from mvngu to roed

comment:5 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:6 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:7 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:8 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:9 Changed 3 years ago by jdemeyer

  • Keywords python3 added

comment:10 Changed 3 years ago by leif

  • Milestone changed from sage-6.4 to sage-7.4

comment:11 Changed 3 years ago by chapoton

  • Branch set to public/14153
  • Commit set to cffdb9077dc8074a0cde0d0cd21d19046b2a95d5

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


New commits:

cffdb90trac 14153 unicode in documentation

comment:12 Changed 2 years ago by git

  • Commit changed from cffdb9077dc8074a0cde0d0cd21d19046b2a95d5 to 1712c87b8afdb2543e593b6e44165aefc52fdef5

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 2 years ago by 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 2 years ago by chapoton (previous) (diff)

comment:14 Changed 2 years ago by 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 2 years ago by chapoton (previous) (diff)

comment:15 Changed 2 years ago by 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 2 years ago by chapoton (previous) (diff)

comment:16 Changed 2 years ago by git

  • Commit changed from 1712c87b8afdb2543e593b6e44165aefc52fdef5 to 8cd268138984060c14b6b43e9ab2604317a9b20a

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

8cd2681trac 14153 adding uft8 declaration in 2 files

comment:17 Changed 2 years ago by chapoton

  • Milestone changed from sage-7.4 to sage-8.0

This should be taken care of.

comment:18 Changed 2 years ago by chapoton

  • Cc embray added

comment:19 Changed 2 years ago by git

  • Commit changed from 8cd268138984060c14b6b43e9ab2604317a9b20a to 489d13f0cd3640a28c05b42077b307173f0c495a

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

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

comment:20 Changed 2 years ago by chapoton

  • Description modified (diff)

comment:21 Changed 2 years ago by chapoton

  • Keywords unicode added

comment:22 Changed 2 years ago by embray

  • Reviewers set to 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 2 years ago by chapoton

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

comment:24 Changed 2 years ago by 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 2 years ago by embray

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 2 years ago by fbissey

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 2 years ago by chapoton

just fails in the same way outside of the patchbot

comment:28 Changed 2 years ago by git

  • Commit changed from 489d13f0cd3640a28c05b42077b307173f0c495a to b9295dca7f1e4c7caa4f6173457a7560f500de40

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

b9295dctrac 14153 encoding required

comment:29 follow-up: Changed 2 years ago by git

  • Commit changed from b9295dca7f1e4c7caa4f6173457a7560f500de40 to be11f06e98fc93888967ae991f4ea91ce33c50e0

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

be11f06trac 14153 fixing some doctests

comment:30 Changed 2 years ago by chapoton

  • Cc jdemeyer hivert added

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

comment:31 Changed 2 years ago by embray

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

comment:32 in reply to: ↑ 29 Changed 2 years ago by embray

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 2 years ago by git

  • Commit changed from be11f06e98fc93888967ae991f4ea91ce33c50e0 to 0c611c5ad8aad31d5403155c40d87f2fb9e087ef

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

0c611c5trac 14153 unicode in documentation

comment:34 Changed 2 years ago by chapoton

  • Branch changed from public/14153 to u/chapoton/14153
  • Commit changed from 0c611c5ad8aad31d5403155c40d87f2fb9e087ef to 080916e00f751500b75fba8c992a7c699ca61ed2

still very broken..


New commits:

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

comment:35 Changed 2 years ago by git

  • Commit changed from 080916e00f751500b75fba8c992a7c699ca61ed2 to f3d375ed3aaa3b2c72c9fa2b30b89ac78d0d85f8

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 2 years ago by git

  • Commit changed from f3d375ed3aaa3b2c72c9fa2b30b89ac78d0d85f8 to 7906a504011eb32f7890c0750d3d987568576655

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

7906a50trac 14153 unicode in documentation

comment:37 Changed 2 years ago by git

  • Commit changed from 7906a504011eb32f7890c0750d3d987568576655 to 1813a63c6e37170300f76df3a8e818b6289b6a63

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

1813a63trac 14153 some details

comment:38 Changed 2 years ago by chapoton

  • Branch changed from u/chapoton/14153 to u/chapoton/14153v2
  • Commit changed from 1813a63c6e37170300f76df3a8e818b6289b6a63 to e3497e851f155914d54174ad18bd28123f6b37a0

here is another tentative


New commits:

e3497e8steps towards unicode doctesting framework

comment:39 Changed 2 years ago by git

  • Commit changed from e3497e851f155914d54174ad18bd28123f6b37a0 to cc03d22738a327999db7162c095daac7d23f9c31

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

cc03d22motorhead detail

comment:40 Changed 2 years ago by git

  • Commit changed from cc03d22738a327999db7162c095daac7d23f9c31 to b091e6a60311ab5756d2fb744e60adabae120a5a

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

b091e6atrac 14153 fixing another doctest

comment:41 Changed 2 years ago by chapoton

There remains only one failing doctest in the r interface.

comment:42 Changed 2 years ago by git

  • Commit changed from b091e6a60311ab5756d2fb744e60adabae120a5a to 8394952acfe919aba99695f84248edbc212a3c9c

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

8394952trac 14153 fixing another doctest

comment:43 Changed 2 years ago by 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 2 years ago by chapoton

  • Authors set to Frédéric Chapoton

comment:45 Changed 2 years ago by chapoton

  • Milestone changed from sage-8.0 to sage-8.1
  • Priority changed from minor to major
  • Status changed from new to needs_review

comment:46 Changed 2 years ago by chapoton

ping, anybody ?

comment:47 Changed 2 years ago by fbissey

Testing on OS X. But it overall looks good.

comment:48 Changed 2 years ago by embray

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 2 years ago by chapoton

ok, well. Then please review.

comment:50 Changed 2 years ago by embray

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 2 years ago by embray

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 2 years ago by embray

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 2 years ago by chapoton

patchbots said green already..

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

Last edited 2 years ago by chapoton (previous) (diff)

comment:54 Changed 2 years ago by git

  • Commit changed from 8394952acfe919aba99695f84248edbc212a3c9c to 4dc084e452d83fe8bf80b105d25b404a21092be7

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 follow-up: Changed 2 years ago by chapoton

  • Status changed from needs_review to needs_work

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

comment:56 Changed 2 years ago by fbissey

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

comment:57 in reply to: ↑ 55 Changed 2 years ago by embray

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 2 years ago by embray

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 2 years ago by 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 2 years ago by chapoton (previous) (diff)

comment:60 Changed 2 years ago by 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 2 years ago by git

  • Commit changed from 4dc084e452d83fe8bf80b105d25b404a21092be7 to 1c0dd43920d5c81b164992fbaf9ecd9f81a3154d

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

1c0dd43trac 14153 trying to fix the gap doc doctests

comment:62 Changed 2 years ago by chapoton

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

comment:63 Changed 2 years ago by embray

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 2 years ago by 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 2 years ago by embray

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 2 years ago by git

  • Commit changed from 1c0dd43920d5c81b164992fbaf9ecd9f81a3154d to 0faa9436561f00cf1ed5b9c8a65ca4667032f2dd

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

0faa943trac 14153 detect gap encoding

comment:67 Changed 2 years ago by chapoton

indeed, here is a try..

comment:68 Changed 2 years ago by chapoton

  • Status changed from needs_work to needs_review

comment:69 Changed 2 years ago by chapoton

bot seems to be stable-green, please review

comment:70 Changed 2 years ago by chapoton

ping ? Jeroen ? Erik ?

comment:71 Changed 2 years ago by vbraun

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 2 years ago by git

  • Commit changed from 0faa9436561f00cf1ed5b9c8a65ca4667032f2dd to ed1e3b5bf17e6cc923e28b11862496dd0e1b9b04

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 2 years ago by 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 2 years ago by chapoton

bot is morally green

comment:75 Changed 2 years ago by roed

  • Reviewers changed from Erik Bray to Erik Bray, Volker Bruan, David Roe
  • Status changed from needs_review to positive_review

Looks good. Sorry that this review took so long!

comment:76 Changed 2 years ago by vbraun

  • Reviewers changed from Erik Bray, Volker Bruan, David Roe to Erik Bray, Volker Braun, David Roe

comment:77 Changed 2 years ago by roed

Sorry for the typo Volker!

comment:78 Changed 2 years ago by vbraun

  • Branch changed from u/chapoton/14153v2 to ed1e3b5bf17e6cc923e28b11862496dd0e1b9b04
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.