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

added jinja2 output handler #371

Merged
merged 3 commits into from Jul 1, 2016

Conversation

Projects
None yet
2 participants
@zacbri
Contributor

zacbri commented Jun 29, 2016

Was there a reason jinja2 was not supported as an output handler ?

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jun 29, 2016

Member

Been meaning to build this one for a while! Thank you for the pull-request. It looks clean, so I will test it out and merge it in shortly.

Member

derks commented Jun 29, 2016

Been meaning to build this one for a while! Thank you for the pull-request. It looks clean, so I will test it out and merge it in shortly.

@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jun 29, 2016

Contributor

Thanks.

I see it fails on python 3.2
Seems to be caused by the % syntax that was removed for strings then re-added in 3.3 (see http://stackoverflow.com/questions/8679985/unicode-strings-syntax-in-python).

How do you want me to handle this ? I can remove the utf8 test if you'd prefer.

Contributor

zacbri commented Jun 29, 2016

Thanks.

I see it fails on python 3.2
Seems to be caused by the % syntax that was removed for strings then re-added in 3.3 (see http://stackoverflow.com/questions/8679985/unicode-strings-syntax-in-python).

How do you want me to handle this ? I can remove the utf8 test if you'd prefer.

@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jun 29, 2016

Contributor

damn... any suggestions?

Contributor

zacbri commented Jun 29, 2016

damn... any suggestions?

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jun 29, 2016

Member

Most likely due to the use of the unicode u prefix. I believe it was removed in Python 3.2, then re-added in 3.3 .... however all strings are treated as unicode in Python 3 so it isn't necessary. For Python 2.x it would be... I'd do something like this: If Python < 3, wrap it in unicode(), if >=3 wrap it in str()?

Member

derks commented Jun 29, 2016

Most likely due to the use of the unicode u prefix. I believe it was removed in Python 3.2, then re-added in 3.3 .... however all strings are treated as unicode in Python 3 so it isn't necessary. For Python 2.x it would be... I'd do something like this: If Python < 3, wrap it in unicode(), if >=3 wrap it in str()?

@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jun 30, 2016

Contributor

Actually, even with that the tests will fail on 3.2.
If you check the output of the last tests, it triggers syntax errors from within Jinja (still unicode). It seems to be caused by Jinja 2.8 being installed as a dependency of Sphinx (needs jinja2>=2.3), but support for Python 3.2 was dropped in Jinja 2.7.

I see 2 options:

  • Explicitly install Jinja2==2.6 in Travis before_script and tune the tests based on Python version as suggested to avoid using unicode strings in 3.2
  • Skip jinja testing for python 3.2 altogether. Rather opinionated I agree...

Let me know what you think

Contributor

zacbri commented Jun 30, 2016

Actually, even with that the tests will fail on 3.2.
If you check the output of the last tests, it triggers syntax errors from within Jinja (still unicode). It seems to be caused by Jinja 2.8 being installed as a dependency of Sphinx (needs jinja2>=2.3), but support for Python 3.2 was dropped in Jinja 2.7.

I see 2 options:

  • Explicitly install Jinja2==2.6 in Travis before_script and tune the tests based on Python version as suggested to avoid using unicode strings in 3.2
  • Skip jinja testing for python 3.2 altogether. Rather opinionated I agree...

Let me know what you think

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jun 30, 2016

Member

Hrm. Ok, will try and check this out today. Won't be the first time we've had to hack around oddities like this. Primary reason to care about Python 3.2 is if any OS Distros still package that version (which I'm not sure which do).

Member

derks commented Jun 30, 2016

Hrm. Ok, will try and check this out today. Won't be the first time we've had to hack around oddities like this. Primary reason to care about Python 3.2 is if any OS Distros still package that version (which I'm not sure which do).

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jun 30, 2016

Member

Looks like Ubuntu 12.04 ships with Python 3.2.x ... getting a Vagrant box setup now to test it there to figure out how to work around it.

Member

derks commented Jun 30, 2016

Looks like Ubuntu 12.04 ships with Python 3.2.x ... getting a Vagrant box setup now to test it there to figure out how to work around it.

@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jun 30, 2016

Contributor

ok I'll give it a go as well

Contributor

zacbri commented Jun 30, 2016

ok I'll give it a go as well

@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jul 1, 2016

Contributor

Hi @derks
I've been trying to setup a dev env on Ubuntu 12.04, but I'm having a hell of a time even installing the dependencies. It looks like most of them aren't supporting python3.2 (at least the versions in Pypi).
I've been able to go with the OS versions for some, but I'm getting stuck on "tabulate".

Any luck on your end ?
BTW, Ubuntu 12.04 is EOL, perhaps it's not that big a deal not to support it ;)

Contributor

zacbri commented Jul 1, 2016

Hi @derks
I've been trying to setup a dev env on Ubuntu 12.04, but I'm having a hell of a time even installing the dependencies. It looks like most of them aren't supporting python3.2 (at least the versions in Pypi).
I've been able to go with the OS versions for some, but I'm getting stuck on "tabulate".

Any luck on your end ?
BTW, Ubuntu 12.04 is EOL, perhaps it's not that big a deal not to support it ;)

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jul 1, 2016

Member

I had the same issue trying to get it running yesterday. I only used Ubuntu 12.04 as an example because it included a version of Python 3.2. The failures shouldn't have anything to do with ubuntu as all the deps are trying to install via PIP, however this all makes me wonder how in the hell Python 3.2 has been passing in Travis CI at all up until this issue? No clue... but it's looking like support for 3.2 might get axed to move forward here. Going to try one more thing, then move on and will merge this in (and drop 3.2 most likely).

Member

derks commented Jul 1, 2016

I had the same issue trying to get it running yesterday. I only used Ubuntu 12.04 as an example because it included a version of Python 3.2. The failures shouldn't have anything to do with ubuntu as all the deps are trying to install via PIP, however this all makes me wonder how in the hell Python 3.2 has been passing in Travis CI at all up until this issue? No clue... but it's looking like support for 3.2 might get axed to move forward here. Going to try one more thing, then move on and will merge this in (and drop 3.2 most likely).

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jul 1, 2016

Member

Should now going through after Issue #372.

Member

derks commented Jul 1, 2016

Should now going through after Issue #372.

@derks derks merged commit e52e0c2 into datafolklabs:master Jul 1, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jul 6, 2016

Contributor

hey @derks
thanks for merging. Do you have any ETA on the next release ?

Contributor

zacbri commented Jul 6, 2016

hey @derks
thanks for merging. Do you have any ETA on the next release ?

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jul 6, 2016

Member

No firm ETA but I always work in clusters... so once I start committing/merging, I generally try to release within a week or two. Also, can you make sure you add yourself to the CONTRIBUTORS file?

Member

derks commented Jul 6, 2016

No firm ETA but I always work in clusters... so once I start committing/merging, I generally try to release within a week or two. Also, can you make sure you add yourself to the CONTRIBUTORS file?

@zacbri

This comment has been minimized.

Show comment
Hide comment
@zacbri

zacbri Jul 6, 2016

Contributor

Ok sounds good, thanks.
I'll make a PR for the CONTRIBUTORS file

Contributor

zacbri commented Jul 6, 2016

Ok sounds good, thanks.
I'll make a PR for the CONTRIBUTORS file

@zacbri zacbri deleted the oasiswork:feat-jinja2-output branch Jul 18, 2016

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