WIP: Flake8 & isort #200

apollo13 · 2017-01-20T18:32:01Z

I copied most of the settings from what we have in Django

spaceone · 2017-01-20T19:21:53Z

circuits/web/parsers/multipart.py

@@ -32,34 +32,18 @@
    THE SOFTWARE.

 '''
-
-__author__ = 'Marcel Hellkamp'


Hm, this is just a package from upstream.

apollo13 · 2017-01-21T09:39:54Z

So, the testsuite passes and I think I've addressed all comments by @spaceone -- any objections to merging that? (Integrating web.url as dependency is imo out of scope for this issue).

spaceone · 2017-01-21T09:43:08Z

You ignored some of my comments about the .decode('unicode_escape') changes

apollo13 · 2017-01-21T09:46:05Z

@spaceone is that not addressed by ae187e6 ?

spaceone · 2017-01-21T09:47:15Z

No, circuits.web.http is affected

spaceone · 2017-01-21T09:47:37Z

And circuits.web.parsers.http

apollo13 · 2017-01-21T09:51:40Z

Mhm, not sure I follow -- I've changed circuits.web.parser.http, but I think I've kept the original behavior of bytes_to_str there. The tests seem also okay (aside from changing them to adhere to the behavior change in b()).

Please tell me what I am missing

spaceone · 2017-01-20T19:26:04Z

circuits/core/pollers.py

@@ -57,7 +59,7 @@ def __init__(self, channel=channel):
    def _create_control_con(self):
        if platform.system() == "Linux":
            return os.pipe()
-        server = create_socket(AF_INET, SOCK_STREAM)
+        server = socket(AF_INET, SOCK_STREAM)


name not already defined?

Nope, no idea why the socket class was aliased to create_socket :D

spaceone · 2017-01-20T19:28:30Z

tests/app/test_daemon.py

+if pytest.PLATFORM == "win32":
+    pytest.skip("Unsupported Platform")
+
+


too much whitespace

Fixed, that said there is plenty of "too much whitespace" still around, but I am not going to fix all of those manually for this PR

spaceone · 2017-01-20T20:06:58Z

circuits/node/utils.py

@@ -13,7 +13,7 @@
 def load_event(s):
    data = json.loads(s)

-    name = bytes_to_str(data["name"].encode("utf-8"))
+    name = data["name"]


please explain

So the current code is: https://github.com/circuits/circuits/pull/200/files#diff-015e99aa1b430ebfaa9aa46322a22df0R16 -- I think that fixes it

Essentially the idea is:

On Py3 classnames are unicode/text which is what json returns, so there is nothing to do

On Py2 classnames need to be bytes so we encode via utf-8

spaceone · 2017-01-20T20:07:51Z

circuits/web/http.py

-try:
-    unicode
-except NameError:
-    unicode = str


please explain

unicode was not used anywhere in this file and I do not think it was added as "utility" for other modules.

spaceone · 2017-01-20T20:08:59Z

circuits/web/parsers/http.py

@@ -184,7 +177,7 @@ def execute(self, data, length):
                else:
                    self.__on_firstline = True
                    self._buf.append(data[:idx])
-                    first_line = bytes_to_str(b("").join(self._buf))
+                    first_line = (b("").join(self._buf)).decode('unicode_escape')


this changes the content. please explain

spaceone · 2017-01-20T20:09:17Z

circuits/web/parsers/http.py

@@ -310,7 +303,7 @@ def _parse_headers(self, data):
            return False

        # Split lines on \r\n keeping the \r\n on each line
-        lines = [bytes_to_str(line) + "\r\n"
+        lines = [line.decode('unicode_escape') + "\r\n"


spaceone · 2017-01-21T09:55:32Z

circuits/web/parsers/http.py

-                    first_line = bytes_to_str(b("").join(self._buf))
+                    first_line = b("").join(self._buf)
+                    if PY3:
+                        first_line = str(first_line, 'unicode_escape')


Okay, this takes a little bit -- lets step back:

What did bytes_to_str(b("").join(self._buf)) originally do:
On py2 bytes_to_str is a no-op:

circuits/circuits/six.py

Lines 299 to 300 in 5797ea3

def bytes_to_str(s):

return s

hence leaving it out is okay for python 2.

On Py3 it did decode the bytes using unicode_escape:

circuits/circuits/six.py

Lines 281 to 282 in 5797ea3

def bytes_to_str(b):

return str(b, "unicode_escape")

which my if PY3 should do.

(bytes_to_str is no longer part [or rather was never] of six, hence I removed it when updating six to upstream)

Arguably the code is broken (I am not sure that unicode_escape is the way to go here), but it is the same brokenness as before (or same correctness for that matter)

Alright. Then it is wrong on python3.

spaceone · 2017-01-21T09:55:47Z

circuits/web/parsers/http.py

@@ -313,7 +305,7 @@ def _parse_headers(self, data):
            return False

        # Split lines on \r\n keeping the \r\n on each line
-        lines = [bytes_to_str(line) + "\r\n"
+        lines = [(str(line, 'unicode_escape') if PY3 else line) + "\r\n"


Same answer as above

prologic added the in progress label Jan 20, 2017

apollo13 changed the title ~~Flake8 & isort~~ WIP: Flake8 & isort Jan 20, 2017

spaceone reviewed Jan 20, 2017

View reviewed changes

apollo13 force-pushed the flake8_isort branch 3 times, most recently from edf3757 to 57ba96d Compare January 20, 2017 20:46

apollo13 mentioned this pull request Jan 21, 2017

Simplified coveralls integration. #203

Merged

spaceone reviewed Jan 21, 2017

View reviewed changes

Flake8 & isort. Also updated six.py while on it.

6240eba

apollo13 force-pushed the flake8_isort branch from ae187e6 to 6240eba Compare January 21, 2017 11:44

apollo13 merged commit f2798d0 into circuits:master Jan 21, 2017

prologic removed the in progress label Jan 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Flake8 & isort #200

WIP: Flake8 & isort #200

apollo13 commented Jan 20, 2017

spaceone Jan 20, 2017

apollo13 commented Jan 21, 2017

spaceone commented Jan 21, 2017

apollo13 commented Jan 21, 2017

spaceone commented Jan 21, 2017

spaceone commented Jan 21, 2017

apollo13 commented Jan 21, 2017

spaceone Jan 20, 2017

apollo13 Jan 21, 2017

spaceone Jan 20, 2017

apollo13 Jan 21, 2017

spaceone Jan 20, 2017

apollo13 Jan 21, 2017

spaceone Jan 20, 2017

apollo13 Jan 21, 2017

spaceone Jan 20, 2017

spaceone Jan 20, 2017

spaceone Jan 21, 2017

apollo13 Jan 21, 2017

spaceone Jan 21, 2017

spaceone Jan 21, 2017

apollo13 Jan 21, 2017

		if pytest.PLATFORM == "win32":
		pytest.skip("Unsupported Platform")

WIP: Flake8 & isort #200

WIP: Flake8 & isort #200

Conversation

apollo13 commented Jan 20, 2017

Choose a reason for hiding this comment

apollo13 commented Jan 21, 2017

spaceone commented Jan 21, 2017

apollo13 commented Jan 21, 2017

spaceone commented Jan 21, 2017

spaceone commented Jan 21, 2017

apollo13 commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment