[WIP] The great style fix of 2019 (+ Python 3 stuff) #792
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This needs a lot more work, including getting off of nose as fast as possible, fixing the style check (which is failing *hard* right now), and finishing Python 3 support, but it gives us a framework to work with. Signed-off-by: Stephen Finucane <stephen@that.guru>
- Remove unnecessary imports - Stop checking for the presence of 'json' - it's been in stdlib since 2.6 [1] - Fix some formatting - Remove invalid 'test_suite' configuration [1] https://docs.python.org/2/library/json.html Signed-off-by: Stephen Finucane <stephen@that.guru>
stephenfin
changed the title
|
It's when I see a PR like this, that I'm really glad that we have tests! |
|
Yup 😅 I've tried to break it up as much as possible but you should definitely consider this WIP until I fix everything up. Tests are currently failing on master for me locally (Fedora 30) so I need to figure that out first. |
This was added to allow the author to "introspect the code without using an IDE". However, it appears no one is using it and it's hacky af. Just remove it. Signed-off-by: Stephen Finucane <stephen@that.guru>
'doc/assets/flowables.py' was an out-of-date copy of 'rst2pdf/flowables.py'. Rather than try keep them in sync, simply add a symlink. Signed-off-by: Stephen Finucane <stephen@that.guru>
This is pretty much a straight swap. Signed-off-by: Stephen Finucane <stephen@that.guru>
They're functionally equivalent and we weren't relying on the iterator behavior anywhere. Signed-off-by: Stephen Finucane <stephen@that.guru>
There's no need to use the wrapper if we're not using keyword arguments. Signed-off-by: Stephen Finucane <stephen@that.guru>
A few remain, but only where aliases have been configured Signed-off-by: Stephen Finucane <stephen@that.guru>
stephenfin
force-pushed
the
python3-ification
branch
from
e621ec8
to
f6d2bb0
Compare
- Remove all commented out config options - Remove everything to do with importing custom extensions manually, which is not something we actually do here - Remove all config options that match Sphinx defaults or are not used in their respective builds, including: templates_path, source_suffix, source_encoding, today, exclude_trees, show_authors, pygments_style, html_*, latex_*, man_* and htmlhelp_* - Fix formatting Signed-off-by: Stephen Finucane <stephen@that.guru>
- Add SPDX license headers [1] - Add 'coding' header (consistently) - Remove unused imports and temporarily disable F403 for the wildcard imports Only production code is fixed. Unit tests will be fixed later. [1] https://spdx.org/sites/cpstandard/files/pages/files/using_spdx_license_list_short_identifiers.pdf Signed-off-by: Stephen Finucane <stephen@that.guru>
I really don't like applying style checkers to existing code bases,
because they tend to make a complete mess of our ability to use
'git-blame' and related tools effectively. With that said, the code base
is in pretty bad shape owing to its sheer age and we'd otherwise be
fixing this stuff for years. Bite the bullet and run all the code
through three tools - black, autoflake8 and isort - meaning we don't
have to worry about this going forward.
isort -rc .
autoflake -r --in-place --remove-unused-variables --expand-star-imports .
black -S .
pre-commit is integrated to ensure this can happen automatically going
forward.
Signed-off-by: Stephen Finucane <stephen@that.guru>
akrabat
changed the title
This way we know exactly what we're using and why. This was done manually like so: ag "class $IMPORT" . .tox/py27/lib/python2.7/site-packages/ The 'rst2pdf/tests/input/pyurl3.py' file is simply excluded from 'flake8' checks since it's clearly not meant to be executed. Signed-off-by: Stephen Finucane <stephen@that.guru>
…sed) Signed-off-by: Stephen Finucane <stephen@that.guru>
This was doing a lot more than is necessary nowadays. Remove it. Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Stephen Finucane <stephen@that.guru>
Disable rules that are too annoying to fix and resolve what's left. Signed-off-by: Stephen Finucane <stephen@that.guru>
stephenfin
force-pushed
the
python3-ification
branch
from
f6d2bb0
to
b04f126
Compare
|
Will be glad when rst2pdf will be working in Python3. Because I am trying to use Sphinx 2.2 instead of 1.8.5, in attempt to get Table-of-Contents links working in PDF file generated by rst2pdf - and Sphinx2.2 is only available with Python3, not Python2. Not sure if it will help my issue with ToC, but... |
|
Current master branch (0.95) already works fine with Python 3 (I have tested it with my Python 3.5). No need for further Python 3 modifications? |
|
And yes, rst2pdf 0.95 (unlike 0.94.1 release) fixes problem with Table of Contents links, thank you! |
See #796 |
|
This is a very courageous thing to do!!! |
johnthagen
reviewed
Sep 18, 2019
| include_package_data=True, | ||
| dependency_links=[ | ||
| ], | ||
| dependency_links=[], | ||
| install_requires=install_requires, | ||
| tests_require=tests_require, | ||
| extras_require=dict( |
There was a problem hiding this comment.
If this PR is the final step for Python 3 compatibility (or at least close enough to call it officially supported), I would also suggest that you add the trove classifiers for Python 3:
Programming Language :: Python :: 3
Programming Language :: Python :: 3.5
Programming Language :: Python :: 3.6
Programming Language :: Python :: 3.7
|
I think we need to do one tool at a time with no other changes in the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I really don't like applying style checkers to existing code bases, because they
tend to make a complete mess of our ability to use 'git-blame' and related tools
effectively. With that said, the code base is in pretty bad shape owing to its
sheer age and we'd otherwise be fixing this stuff for years. Bite the bullet and
run all the code through three tools - black, autoflake8 and isort - meaning we
don't have to worry about this going forward.