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
Removal of smartypants #708
Conversation
@akrabat our old copy of smartypants has ... 572 style and other kinds of flake8 warnings, so this looks like a good thing to merge :-) |
I can't seem to run the tests:
However smartypants is installed:
|
Grmbl, I will take a look at that in a second.
…On Wed, Jan 2, 2019 at 4:38 PM Rob Allen ***@***.***> wrote:
I can't seem to run the tests:
$ nosetests -x -v -i regulartest -i sphinxtest
numbered_links.txt ... FAIL
======================================================================
FAIL: numbered_links.txt
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/rob/.pyenv/versions/2.7.15/envs/rst2pdf/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
self.test(*self.arg)
File "/Users/rob/Projects/python/rst2pdf/rst2pdf/tests/test.py", line 50, in __call__
assert key == 'good', '%s is not good: %s'%(f,key)
AssertionError: /Users/rob/Projects/python/rst2pdf/rst2pdf/tests/input/numbered_links.txt is not good: fail
-------------------- >> begin captured stdout << ---------------------
Process "coverage" started on Wed Jan 2 19:36:44 2019
coverage run -a /Users/rob/.pyenv/versions/rst2pdf/bin/rst2pdf
--date-invariant -v numbered_links.txt --use-numbered-links -o
/Users/rob/Projects/python/rst2pdf/rst2pdf/tests/output/numbered_links.pdf
* Traceback (most recent call last):
* File "/Users/rob/.pyenv/versions/rst2pdf/bin/rst2pdf", line 11, in <module>
* load_entry_point('rst2pdf', 'console_scripts', 'rst2pdf')()
* File "/Users/rob/.pyenv/versions/2.7.15/envs/rst2pdf/lib/python2.7/site-packages/pkg_resources/__init__.py", line 484, in load_entry_point
* return get_distribution(dist).load_entry_point(group, name)
* File "/Users/rob/.pyenv/versions/2.7.15/envs/rst2pdf/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2714, in load_entry_point
* return ep.load()
* File "/Users/rob/.pyenv/versions/2.7.15/envs/rst2pdf/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2332, in load
* return self.resolve()
* File "/Users/rob/.pyenv/versions/2.7.15/envs/rst2pdf/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2338, in resolve
* module = __import__(self.module_name, fromlist=['__name__'], level=0)
* File "/Users/rob/Projects/python/rst2pdf/rst2pdf/createpdf.py", line 99, in <module>
* from rst2pdf.nodehandlers import nodehandlers
* File "/Users/rob/Projects/python/rst2pdf/rst2pdf/nodehandlers.py", line 10, in <module>
* from . import genelements
* File "/Users/rob/Projects/python/rst2pdf/rst2pdf/genelements.py", line 52, in <module>
* from .basenodehandler import NodeHandler
* File "/Users/rob/Projects/python/rst2pdf/rst2pdf/basenodehandler.py", line 40, in <module>
* from smartypants import smartypants
* ImportError: cannot import name smartypants
Program coverage exit code: FAIL (1) elapsed time: 01.6 seconds
File numbered_links.pdf not generated
However smartypants is installed:
$ pip show smartypants
Name: smartypants
Version: 2.0.1
Summary: Python with the SmartyPants
Home-page: https://github.com/leohemsted/smartypants.py
Author: Leo Hemsted
Author-email: ***@***.***
License: BSD License
Location: /Users/rob/.pyenv/versions/2.7.15/envs/rst2pdf/lib/python2.7/site-packages
Requires:
Required-by:
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#708 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAGK_vCK2LPlFWAkdAYuGTiS30VHU4nks5u_QqqgaJpZM4YXtQL>
.
|
@akrabat That error importing smartypants is because you have stale .pyc files, remove them and all will be well (although there are other test errors, which I am looking into) |
@akrabat please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end the amount of code changed is surprisingly very little!
Yeah, there was just a minor API change in smartypants in the last ... 10
years or so?
…On Thu, Jan 3, 2019 at 1:15 PM Oz N Tiram ***@***.***> wrote:
***@***.**** approved this pull request.
In the end the amount of code changed is surprisingly very little!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#708 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAGK9ff7wuYVdzydX5p5P5nVknE7vtbks5u_iyvgaJpZM4YXtQL>
.
|
@@ -39,4 +39,9 @@ svg2rlg==0.3 | |||
typing==3.6.6 # via sphinx | |||
urllib3==1.24.1 # via requests | |||
webencodings==0.5.1 # via html5lib | |||
smartypants==2.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to go in requirements.in
I believe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eek, what the heck is pip-compile? It doesn't even seem to exist anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
rst2pdf/basenodehandler.py
Outdated
@@ -36,10 +36,11 @@ | |||
import inspect | |||
import types | |||
|
|||
from log import log, nodeid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate of line 44
rst2pdf/createpdf.py
Outdated
@@ -94,7 +94,7 @@ | |||
from rst2pdf.image import MyImage, missing | |||
from rst2pdf.aafigure_directive import Aanode | |||
from rst2pdf.log import log, nodeid | |||
from rst2pdf.smartypants import smartyPants | |||
import smartypants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we do from smartypants import smartypants
as per basenodehandler.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same thing, but sure, why not.
rst2pdf/createpdf.py
Outdated
self.smarty = smarty | ||
|
||
# See https://pythonhosted.org/smartypants/reference.html#smartypants-module | ||
self.smarty = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this property to smartypants_attributes
so that we know it's not the same as the smarty
variable which holds the command line value of (0, 1 ,2 or 3)?
@@ -1 +1 @@ | |||
--smart-quotes=1 | |||
--smart-quotes=2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a BC break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because the change in smartypants APIs don't allow reproducing the old behaviour, because there is no way to reproduce the specific combination of transformations it used to do.
IIRC This seems to have been broken, so I changed the behaviour and the
docs to match what was the actual behaviour.
…On Thu, Jan 3, 2019 at 5:51 PM Rob Allen ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In rst2pdf/tests/input/test_smarty.cli
<#708 (comment)>:
> @@ -1 +1 @@
---smart-quotes=1
+--smart-quotes=2
Is this a BC break?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#708 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAGKzzoaR1OL2UoyRqIHUr98xYC2hW7ks5u_m0-gaJpZM4YXtQL>
.
|
You were right.
|
@akrabat I think I addressed all the comments, merge at will :-) |
Closes rst2pdf#708 Fixes rst2pdf#694
Removed our copy of Smartypants and added it as a dependency. Fix #694