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

Explicitely convert bytes to str. Works in both Python 2 and 3, Fixes… #204

Merged
merged 2 commits into from Apr 4, 2016

Conversation

Projects
None yet
2 participants
@marios-zindilis
Copy link
Contributor

marios-zindilis commented Apr 3, 2016

#203

See issue #203 for the problem. This patch works in both Python2 and Python3, fixes #203.

@nicholasserra

This comment has been minimized.

Copy link
Collaborator

nicholasserra commented Apr 4, 2016

Thank you!

@nicholasserra nicholasserra merged commit 82eb31b into trentm:master Apr 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@marios-zindilis

This comment has been minimized.

Copy link
Contributor

marios-zindilis commented Apr 4, 2016

The second commit, titled "Improve performance of carriage return replacement." was not meant to be in the same Pull Request, which is why it's not documented. My poor git skills betrayed me. Here is the explanation.

String manipulation with Python's re module is usually slower than achieving the same effect without using re. The re2 library is a significantly faster in-place replacement for re, but would create a dependency for the core functionality of python-markdown2, which I don't think is desirable.

A brief look at this specific example (carriage return replacement) can be found here, and for completeness:

In [1]: timeit 'Apples\r\nOranges\r\nKiwis\rGrapes\r'.replace('\r\n', '\n').replace('\r', '\n')
1000000 loops, best of 3: 404 ns per loop

In [2]: import re

In [3]: timeit re.sub(r'\r\n|\r', '\n', 'Apples\r\nOranges\r\nKiwis\rGrapes\r')
100000 loops, best of 3: 2.03 µs per loop

So re is about 5 times slower than str.replace(). Of course this is a negligible improvement in the total execution time of the conversion, but in time I'd like to look at more performance improvements for python-markdown2.

@nicholasserra

This comment has been minimized.

Copy link
Collaborator

nicholasserra commented Apr 4, 2016

Sounds good, thanks!

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