Python 2 Support about panflute HOT 51 CLOSED

I think it's a regression to make efforts for supporting Python 2.

In the Python 2.7 Countdown you can see that Python 2 will not be maintained past 2020. So, IMO it's no time to make Python 3 apps retrocompatible, but to upgrade old apps to version 3.

from panflute.

This requires some additional testing, and I haven't been able to get around to it yet. I'll close this issue once more progress is made.

from panflute.

After the fix in #35, Build #63 - ickc/panflute - Travis CI passes "every versions" except 2.6 and 3.2. i.e. including 2.7 and pypy2.

Would it be possible to have pasteurize as a dependency of setup.py? So with a specific flag we can build a pasteurized egg?

I googled that before, and found What is the conventional way to automatically run pasteurize against a python 3 project? - Stack Overflow: it is just a question without answer. So if it can be done, at least it is not commonly done/known.

One possible way is to use the Setup Configuration File. A subfolder can be created in the master branch which include the pasteurized version. And an alternative .cfg can be setup to use that folder instead. But the downside are

1. not as simple as pip install...
2. polluting the master branch since the generated "duplicates" is committed into it directly.

Currently, the way I did it in ickc/panflute at py2-3 is to pasteurize before tests and deploy in Travis. So deploying the pasteurized version to PyPI is trivial. With some modifications, Travis can be setup to commit the pasteurize version to a certain branch instead.

I asked in pandoc-discuss, but no one has replied about the need of a Python 2 compatible version though.

from panflute.

At this point writing a port to Py2 would probably involve at least some work:

• The io code needs to be changed
• Tuple unpacking would need to be changed as I think it's not supported on 2.7
• More importantly, the code that navigates around the tree (and ensures its stable) relies heavily on collections (OrderedDict, MutableSequence, MutableMapping) that might have changed from 2 to 3.

For me it's not worth it to put the hours as I would not benefit from it, but if someone submits a PR that adds support for Py2 I would almost surely accept it.

Cheers,
S

from panflute.

Thanks for the update. I didn't realize the port would now take this much work, and then it makes sense to keep it to Python 3.

from panflute.

I have a version that is close to working. quick.py tests are all running. The tests in the run_tests.py is failing currently because of a unicode write issue in json dumps. I'm looking into this right now, but if you have an idea on how to solve this that would be great.

from panflute.

I don't have detailed enough understanding of where this package converts to and from unicode, i.e. the boundaries of unicode. My guess is that the Python3 version works because everything is unicode, however the Python2 doesn't because there's strings and unicodes littered around. Additionally, there may be another problem when working with the json module in Python2. This StackOverflow answer describes the odd behaviour with json.dumps that seems to be the source of the issue that I've been wrestling with all morning. And the post recommends the best way to avoid this problem is to use Python3 😄 .

I was able to run run_tests.py after modifying the code to open the output file in binary mode. The tests all "pass" (files are of the same length etc). However, I was only able to do this using a very small Markdown file converted to the AST json by Pandoc.

I also tested the current code base with Python3 and it works fine, so I'm assuming the changes I introduced did not break anything. I can submit a pull request at this stage, but there's probably one change in the io.py file with respect to json.dumps or a few unicode related changes that need to happen before this works across Python versions. Most of the changes are minimal. The use of six, future and other packages made backporting fairly straightforward. However, these changes may not be aesthetically pleasing to someone who has only used Python3 (this version makes extensive use of kwargs - this will not allow introspecting the function signature). But, since this package is extremely well documented (great job @sergiocorreia!), I think this a worthwhile compromise, in return gaining Python2/3 support.

from panflute.

By the way, about the unicode problem with run_tests.py: perhaps the problem is that run_tests.py has some code specific for python 3? I vaguely remember that the args of open() used to matter in Python 2 (e.g. I had to use mode='wb' on windows), so it might be worth to explore.

from panflute.

Thanks for the suggestion! I tried that as well. I'm using the open from future.builtins and tried opening the file with mode=wb but got a different error when I tried that. Also, when I printed the document using print(doc.to_json()) there were some "strings" in there ( e.g. "UnMeta", "t", "c" ) that were str types, and other "strings" that were of type unicode (e.g. u"word"). The unicode strings were strings that came from the Markdown file I'd used. I also tried adding a decode for various tags and types but that did not solve the problem.

I'm listing all of this here so that I can use it as reference later. It'll also be great if someone knows a quick fix or offer a suggestion. I'll probably get a chance to take a look at this later this week.

from panflute.

Cool! Let me know how you feel about maintaining a py2 branch. My hands are a bit full right now and I think it would be better if someone that's using py2 to maintain that branch.

from panflute.

Probably a bit late to ask this: should the master branch be modified to be dual compatible with Python 2 & 3 rather than keeping it as 2 separate branch? This way the master is a bit uglier but then we don't need to keep the 2 branches in sync.

from panflute.

from panflute.

should the master branch be modified to be dual compatible with Python 2 & 3 rather than keeping it as 2 separate branch?

We thought about this and doing so would end up with a very messy codebase. Moreover, as Dheepak said, having a Py2 branch is more important than having it integrated with Py3, so for now I wouldn't worry about this.

from panflute.

@ickc There's more information in this discussion #6 if you are interested.

from panflute.

Are passing the 4 pytest ok or are there other things to worry?

from panflute.

The tests are not comprehensive at all, so they would give you a wrong sense of confidence (and writing a complete set of tests would require a lot of work). Moreover, even if we increase the test coverage, some input would fail on Py2 (like unicode input).

from panflute.

I am thinking a quick way to add some tests are to copy the pandoc tests files in pandoc/tests at master · jgm/pandoc. Then may be run some filters (e.g. from your examples) against those files. Some filters might be written only for the purpose of testing things. That would add an "unknown amount of coverage".

from panflute.

By the way, have you tried pasteurize?

from panflute.

I haven't tried it. I tried a few things at some point (six perhaps?) but quickly gave up due to lots of small complications.

Adding those tests might work. There are also lots of panflute-specific things that of course won't get tested unless we know what to look for (like convert_text, yaml filters, autofilters, etc.).

For instance, in the case of the autofilters submodule, it's hard to write a standard unit tests because if you run it from the command line (pandoc ... -F panflute) you could trigger bugs that for you would not get if you somehow just run it from python directly (specially when dealing with unicode).

from panflute.

The tests are not comprehensive at all

I ran coverage and it says the coverage is at 79%, so not bad?

I have a branch in ickc/panflute at py2-3 that passes py.test on Python 2.7, and 3.3-5. The commit responsible to the change made in Python is in pasteurize and manual fixes · ickc/panflute@443dd10, which uses pasteurize and some manual fixes.

May be some tests can be added to check the potential breaking points, like the io mentioned?

The only major code changes done in that commit are the unicode vs str and *args, **kv. So it doesn't make the code much uglier generally. So may be it needs not be separated into 2 branches?

Travis build is in Build #33 - ickc/panflute - Travis CI.

from panflute.

My main concern is that a pasteurized version increases complexity (e.g. the commit has 222 extra lines), and down the line when we want to drop Py2 compatibility (in 3 years or so) we would be stuck with these extra lines.

If there's a way to pasteurize and patch it up only when packaging (i.e. when running setup.py) that would be great. Of course, that might create extra problems (a user might find a bug in a line not present in the github version).

That's why for now I would prefer it to stay in a py2 branch, to minimize developer costs (heck, even supporting Pandoc 1.7.2 and Pandoc 1.8 is taxing enough for me).

from panflute.

Even if putting it on another branch, I think a better way to do will be:

1. auto generate the py2-3 branch like I did
2. For things that pasteurize doesn't work, edit the master branch rather than the generated branch. Then generate again.

In this case the one requires manual editing is about 10 lines, much more manageable with minimum "pollution" if merged into the master branch.

So in principle, the only branch to maintain will be the master branch. And if we want to be fancy, we can setup Travis to deploy the generated branch.

By the way, even if the generated branch is for py2 only, targeting py2-3 might be beneficial in a few ways:

1. if the end-users accidentally installed the py2-3 source using python3, in principle it should still works.
2. we could actually distribute the py2-3 version in PyPI, and keep the master branch for development. This way, when we want to drop py2 support, the master branch is still (almost) clean. What's needed to change is the PyPI submission script.

from panflute.

the commit has 222 extra lines

On the other hand, even if the generated code is merged into the master branch, most of them are import statements though. It shouldn't be too hard to purge them. The only "real" additional code are in panflute/base.py line 90-94, and a bunch of codes dealing with **kwargs in panflute/containers.py, panflute/elements.py.

from panflute.

Yes, I agree these are some nice benefits. It would be important to know what are the 10 lines that would require a manual fix, to see if we can put them in master.

About your last point:

On the other hand, even if the generated code is merged into the master branch, most of them are import statements though

Yes, but it's still code, and it still makes it harder to read 6 months down the line. The more complex the code, the harder it is for someone else to fix a bug or make a PR, or even for me to grok what's going on.

from panflute.

Here are the ones I can remember:

• panflute/base.py at 443dd108db2c2fffbd0de9ad395a2e22752b9e4a · ickc/panflute

• panflute/base.py at 443dd108db2c2fffbd0de9ad395a2e22752b9e4a · ickc/panflute

• panflute/tools.py at 443dd108db2c2fffbd0de9ad395a2e22752b9e4a · ickc/panflute

from panflute.

Premature sent the previous message (some combination of key presses resulted in send).

In the hindsight, I should have separated the auto vs. manual in 2 commits. I was thinking about not committing until it passes the tests...

And I agree to keep the master clean, for the bunch of **kwargs, they are unreadable.

I think the better approach will be using master as "source code" and the generated code to push to pypi. I think if done correctly, it does not require another branch: just test and deploy the generated code instead of the "source code". But a separate branch might still be desirable e.g. for bug reports.

from panflute.

Makes sense. Feels a bit cumbersome but if it means keeping the source code clean it might just work.

from panflute.

Does any of you have some files that is known to cause problems in the Python 2 branch? I'm thinking about add a quick test using filecmp to see if my conversion works.

from panflute.

Any update on the thoughts about this? e.g. generating the "pypi" branch for 2-3 compatibility; test files that known to cause problems with panflute on python 2.

from panflute.

I don't even have py2 installed on my laptop so have no easy way of testing. If you manage to get it working with the markdown files in the test folder, and a few filters, then we should be good to go

from panflute.

After thinking about it for some time, I think it is still easier to commit minimally to the master branch, and using that to auto-generate (pasteurize) the python 2-3 version. It means that the master branch will be modified, but only in cases pasteurize cannot handle. The biggest advantage of this approach is one need not to maintain 2 branches separately and keep pasteurizing it and merge.

I have a branch that's almost working now: ickc/panflute at python-2-and-3, except that I hit issue #35. You can compare the code from the master branch here, and really not a lot of lines has been added. And whenever things are added, a boolean py2 is used, so a simple grep can be used to hunt down these codes when we finally decided to kill Python2 support.

I think the remaining question is, should the pasteurized version be submitted to PyPI, or should it be auto-commit by Travis to a separate branch:

• Pros of pasteurized to PyPI: uniform installation experience, not having a separate branch (that commits by machine)
• Cons: When error occur, the line no. reported will not match the one in master branch. In order to trace the line no., one has to run pasteurize and see where that line actually is. If the code is in a separate branch already, it is very easy to find by going to GitHub web interface.

from panflute.

Would it be possible to have pasteurize as a dependency of setup.py? So with a specific flag we can build a pasteurized egg?

This would give us a bit more flexibility, so whenever we want to run setup.py , we can choose to install the py3 version or the pasteurized version (might help when debugging).

Also, we can use that flag when building with travis, and then as you mention Travis can push the pasteurized branch to PyPi

from panflute.

Having it in Travis sounds reasonable. In that way, master would be mostly free of python2 oriented code, which would reside in a specific branch and on PyPi as pasteurized code.

Feel free to submit a PR if you feel confident with the tests

from panflute.

which would reside in a specific branch and on PyPi as pasteurized code

Do you mean in addition to deploying to PyPI with the pasteurized code, you want to have a separate branch to contain the pasteurized code as well?

Feel free to submit a PR if you feel confident with the tests

About the test: I mentioned about processing all .md files in the master branch with the panflute filter applied (and hence auto-filter will applies the example filters). Previously I was sure the exit code is 0, but did not guarantee any gibberish result.

So I created one more test in Release Testing if the py2 and py3 output are identical · ickc/panflute, to test if the native output by py2 and py3 panflute are identical. And they are, all the native output from different python version variants are identical.

Since the .md and .py filter examples in the repository are fairly diversify, I think that is a pretty good indicator the py2 version is working well.

from panflute.

The separate branch might not be necessary. Also, in case of bug reports, we could ask them to run them against the version in master which would ameliorate any potential confusion about the line of the bug.

Feel free to add the PR, as well as the tests of course.

Cheers,
S

from panflute.

There's quite a few commits devoted to testing the Travis setup. Currently my branch is about 30 commits ahead of yours. Do you want me to clean it up, or is it ok to leave as is? The commit history looks like this.

from panflute.

I checked the code yesterday and it looked down, but agree about the commits. Maybe one PR for the py2 or another for Travis? Whatever works best on your side

from panflute.

I checked the code yesterday and it looked down, but agree about the commits. Maybe one PR for the py2 or another for Travis? Whatever works best on your side

I don't understand. Can you elaborate?

By the way, for the different pandoc versions, do you want to test against 1.17.2, 1.18, latest; or just 1.17.2 and latest?

from panflute.

What I meant is that I really care about the autocompletion, so much that I ditched the initial version of panflute. From #6, @sergiocorreia

Can you elaborate more on this point? Which change for py2 compatibility would affect this?

from panflute.

By the way, for the different pandoc versions, do you want to test against 1.17.2, 1.18, latest; or just 1.17.2 and latest?

Note: pandoc 2.0 will be out soon. So it might be better to keep one known working "pandoc >=1.18" and a "latest" separately, and put the "latest" in "allow_failure". On the other hand, we might as well do that only when pandoc 2.0 is breaking something.

from panflute.

What I meant is that I really care about the autocompletion, so much that I ditched the initial version of panflute
Can you elaborate more on this point?

When you are writing panflute code on an editor (e.g on IDLE) you get some nice autocompletion, such as:

This is lost if the arguments are just *args or something noninformative:

But this seems to be a non--issue because I checked it out and autocompletion works as before on 1.8

from panflute.

About the pandoc versions, I would keep 1.17.2 (a lot of people seem to have it, and if it works in that version it also probably works in 1.16 and so on). ~~~On top of that, 1.8 and latest (this one with allow_failure of course)~~~

Edit: on second thought, I think 1.17.2 and latest should be enough

from panflute.

This is lost if the arguments are just *args or something noninformative:

This is what I worried. The pasteurized version will only gives *args, **_3to2kwargs.

But this seems to be a non--issue because I checked it out and autocompletion works as before on 1.8

You must be running the un-pasteurized version then.

As long as you install via pip install git+git://github.com/sergiocorreia/panflute.git, then you'd get the un-pasteurized version. But for the current pull request #36, pip install panflute will always gives you the pasteurized version no matter the end-user is using py2 or py3.

So it depends which one should be in PyPI.

1. Either you want a pasteurized one in PyPI so every end-users (including py2 users) can install just by pip install panflute, but Python3 users care about the autocompletion you designed for has to do pip install git+git://github.com/sergiocorreia/panflute.git.

2. Or the un-pasteurized version in PyPI, but every py2 end-users has to install from a separate branch in this repository, say, py2-3.

from panflute.

sergiocorreia closed this in e14db4f 2 minutes ago

I wish I typed faster...

from panflute.

I see your point. I won't revert the commit, but if you find out a way to allow py3 users to have autocompletion, let me know (I googled for ways to have two versions on pypi, but couldn't find anything)

from panflute.

For now, I think the README need to add that alternative installation method.

I'll keep exploring for a better setup. It seems the current way sacrifice too much for a minority of py2 users. But on the other hand, filter authors should be the minority comparing to the filter users, and the later one might not want to care what's the python version they are using (Mac default ships with Python2 only).

from panflute.

I think Python wheel should do the trick. If it works, I probably will do it in a few days.

from panflute.

One of the technical difficulties of using wheel (to have separate codes for Python2 and Python3) is to use a VM image in Travis that has both Python2 and Python3 available. It might means a Tox setup is needed. If so it means the Travis setup might be quite different from what we are using now (build time might be longer too).

Such setup should be very similar to a project that write in py2 and uses wheel to distribute a different binaries for py3 from 2to3. If anyone knows a project setup Travis to do so, let me know and we can just modify from there.

from panflute.

I'll see if I can find anything with a similar setup. If not, we'll leave this as it is for now.

from panflute.

Hi, all,

Sorry for everyone. About 3 hours ago, I made a pull request that uses wheel to support py2, which should be an improvement over the previous setup (pasteurized version no longer affect py3 users through pip).

However, for a couple of reasons, the new update will break py2 support on panflute 1.9.7 and 1.10. A new pull request #54 is supposed to fix this (and I asked @sergiocorreia to remove the problematic binaries in 1.9.7 and delete 1.10 in PyPI). For the meanwhile, py2 users should not use panflute 1.9.7 and 1.10. If you have py2 packages depending on panflute, add install_requires=['panflute!=1.9.7, !=1.10'] to your setup.py.

I am sorry for the inconvenience.

from panflute.

Hi, all,

In short, the latest 1.10.2 is working, and has a separate wheel for py2 and py3 users. So the py3 users would get the unpasteurized version (i.e. identical to the source code).

@sergiocorreia has deleted the problematic 1.9.7 and 1.10 versions on PyPI. So you don't need to ignore these 2 versions in your setup.py.

This incidence is a combination of stupid mistake, and that I cannot fully tested the Travis+PyPI setup (I tested the "same" setup on pantable before the release, but here's the "stupid mistake" above: I forgot to check setup.cfg). And then the delays on the merge of the fix is because I was doing too much pull request and commits at the same time (that causes @sergiocorreia's Travis very busy, so even if @sergiocorreia approved it, he didn't have a chance to merge before he got busy/went to sleep).

Sorry again, but I hope that this current setup will finally done away the compromises between py2 and py3 users, while making maintaining both relatively easy.

from panflute.