#30964 closed enhancement (fixed)

partial flake8 cleanup for repl/

Reported by: chapoton Owned by:
Priority: minor Milestone: sage-9.3
Component: refactoring Keywords:
Cc: tscrim, slelievre Merged in:
Authors: Frédéric Chapoton Reviewers: Samuel Lelièvre
Report Upstream: N/A Work issues:
Branch: da3885d (Commits, GitHub, GitLab) Commit: da3885d686dd4ae1478bf7d0fe0fa85124c2d572
Dependencies: Stopgaps:

Status badges

Description

as a general cleaning procedure.

Change History (20)

comment:1 Changed 13 months ago by chapoton

  • Branch set to u/chapoton/30964
  • Commit set to 64efc98152ca15ea6ec4e6cc7abd1636ff9504cc
  • Status changed from new to needs_review

New commits:

64efc98some flake8 for repl

comment:2 Changed 12 months ago by chapoton

  • Cc tscrim slelievre added

ok, bot is morally green, please review

comment:3 Changed 12 months ago by slelievre

Below some "since-we-are-editing-these-lines" optional changes.

Clearly out of scope for this reformatting ticket, so feel free to make them or not.

With all or some or none of these in, you can set to positive_review on my behalf.

Optionally fix these comments:

-# TODO: This global variable do_preparse should be associated with an
+# TODO: This global variable _do_preparse should be associated with an
 # IPython InteractiveShell as opposed to a global variable in this
 # module.
 _do_preparse = True
-    If the file is not a Cython, Python, or a Sage file, a ``ValueError``
+    If the file is not a Cython, Python, or Sage file, a ``ValueError``
-    # Note: On Python 3 b64encode only accepts bytes, and returns bytes (yet
-    # b64decode does accept str, but always returns bytes)
+    # Note: In Python 3, b64encode only accepts bytes, and returns bytes.
     b64 = base64.b64encode(str_to_bytes(filename, FS_ENCODING,
                                         "surrogateescape"))
     return 'sage.repl.load.load(sage.repl.load.base64.b64decode("{}"),globals(),{})'.format(bytes_to_str(b64, 'ascii'), attach)

Optionally split a long line and pep8 its output:

-    return 'sage.repl.load.load(sage.repl.load.base64.b64decode("{}"),globals(),{})'.format(bytes_to_str(b64, 'ascii'), attach)
+    return ('sage.repl.load.load(sage.repl.load.base64.b64decode'
+            '("{}"), globals(), {})'.format(bytes_to_str(b64, 'ascii'), attach))

Optionally shrink two lines into one (unless it decreases readability):

-        i = s.find('\n')
-        s = s[i + 1:]
+        s = s[s.find('\n') + 1:]

comment:4 Changed 12 months ago by git

  • Commit changed from 64efc98152ca15ea6ec4e6cc7abd1636ff9504cc to e17445f61f4e411e3309b91987bc60c387c13140

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

e17445fminor details

comment:5 Changed 12 months ago by slelievre

  • Reviewers set to Samuel Lelièvre
  • Status changed from needs_review to positive_review

comment:6 Changed 12 months ago by slelievre

Regarding the two flake8 complaints of patchbots.

One says

src/sage/repl/interface_magic.py:127:21 undefined name 'get_ipython'

but that seems unrelated to the changes here.

One says

src/sage/repl/interpreter.py:788:13 'line_profiler' imported but unused

but that is as intended: it is imported in a try-except-else, only to ascertain and record whether it is available:

        # Load the %lprun extension if available
        try:
            import line_profiler
        except ImportError:
            pass
        else:
            self.extensions.append('line_profiler')

comment:7 Changed 12 months ago by git

  • Commit changed from e17445f61f4e411e3309b91987bc60c387c13140 to fe0908e7312480aefdaead03fc7f2af877c114eb
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

fe0908eMerge branch 'u/chapoton/30964' in 9.3.b3

comment:8 Changed 12 months ago by chapoton

  • Status changed from needs_review to positive_review

after trivial rebase, setting back to positive

comment:9 Changed 12 months ago by fbissey

  • Status changed from positive_review to needs_work

Ouch that hurts

 * python3_8: running distutils-r1_run_phase python_compile_all
Traceback (most recent call last):
  File "sage_setup/docbuild/__main__.py", line 1, in <module>
    from sage_setup.docbuild import main
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/sage_setup/docbuild/__init__.py", line 57, in <module>
    import sage.all
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/build/lib/sage/all.py", line 131, in <module>
    from sage.misc.all       import *         # takes a while
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/build/lib/sage/misc/all.py", line 30, in <module>
    from .html import html
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/build/lib/sage/misc/html.py", line 19, in <module>
    from sage.misc.latex import latex
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/build/lib/sage/misc/latex.py", line 28, in <module>
    from sage.misc import sage_eval
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/build/lib/sage/misc/sage_eval.py", line 14, in <module>
    import sage.repl.preparse as preparser
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_8/build/lib/sage/repl/preparse.py", line 1924
    <<<<<<< HEAD

Someone left out git stuff from a merge in sage/repl/preparse.py.

comment:12 Changed 12 months ago by vbraun

There are some merge conflict markers checked in %-)

comment:13 Changed 12 months ago by git

  • Commit changed from fe0908e7312480aefdaead03fc7f2af877c114eb to fd7416d707d8c4e09c40b61d1a7d0dda9a770005

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

fd7416dsome flake8 for repl

comment:14 Changed 12 months ago by chapoton

  • Status changed from needs_work to needs_review

ok, sorry. Here is a clean new branch

comment:15 Changed 12 months ago by chapoton

please review

comment:16 Changed 12 months ago by fbissey

  • Status changed from needs_review to positive_review

Looks clean. Back to positive.

comment:17 Changed 12 months ago by vbraun

  • Status changed from positive_review to needs_work

merge conflict

comment:18 Changed 12 months ago by git

  • Commit changed from fd7416d707d8c4e09c40b61d1a7d0dda9a770005 to da3885d686dd4ae1478bf7d0fe0fa85124c2d572

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

da3885dMerge branch 'u/chapoton/30964' in 9.3.b4

comment:19 Changed 12 months ago by chapoton

  • Status changed from needs_work to positive_review

after trivial rebase, setting back to positive

comment:20 Changed 11 months ago by vbraun

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