Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

partial flake8 cleanup for repl/ #30964

Closed
fchapoton opened this issue Nov 25, 2020 · 26 comments
Closed

partial flake8 cleanup for repl/ #30964

fchapoton opened this issue Nov 25, 2020 · 26 comments

Comments

@fchapoton
Copy link
Contributor

as a general cleaning procedure.

CC: @tscrim @slel

Component: refactoring

Author: Frédéric Chapoton

Branch/Commit: da3885d

Reviewer: Samuel Lelièvre

Issue created by migration from https://trac.sagemath.org/ticket/30964

@fchapoton fchapoton added this to the sage-9.3 milestone Nov 25, 2020
@fchapoton
Copy link
Contributor Author

New commits:

64efc98some flake8 for repl

@fchapoton
Copy link
Contributor Author

Commit: 64efc98

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/30964

@fchapoton
Copy link
Contributor Author

comment:2

ok, bot is morally green, please review

@slel
Copy link
Member

slel commented Nov 28, 2020

comment:3

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:]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 28, 2020

Changed commit from 64efc98 to e17445f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 28, 2020

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

e17445fminor details

@slel
Copy link
Member

slel commented Nov 28, 2020

Reviewer: Samuel Lelièvre

@slel
Copy link
Member

slel commented Nov 28, 2020

comment:6

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')

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

Changed commit from e17445f to fe0908e

@fchapoton
Copy link
Contributor Author

comment:8

after trivial rebase, setting back to positive

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2020

comment:9

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.

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2020

@vbraun
Copy link
Member

vbraun commented Dec 7, 2020

comment:12

There are some merge conflict markers checked in %-)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 8, 2020

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

fd7416dsome flake8 for repl

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 8, 2020

Changed commit from fe0908e to fd7416d

@fchapoton
Copy link
Contributor Author

comment:14

ok, sorry. Here is a clean new branch

@fchapoton
Copy link
Contributor Author

comment:15

please review

@kiwifb
Copy link
Member

kiwifb commented Dec 11, 2020

comment:16

Looks clean. Back to positive.

@vbraun
Copy link
Member

vbraun commented Dec 13, 2020

comment:17

merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 15, 2020

Changed commit from fd7416d to da3885d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 15, 2020

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

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

@fchapoton
Copy link
Contributor Author

comment:19

after trivial rebase, setting back to positive

@vbraun
Copy link
Member

vbraun commented Dec 27, 2020

Changed branch from u/chapoton/30964 to da3885d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants