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

Support Python 3 #348

Closed
disko opened this Issue Nov 22, 2014 · 24 comments

Comments

Projects
None yet
5 participants
@disko
Copy link
Member

disko commented Nov 22, 2014

Title says it all.

A quick check of requirements.txt with caniusepython3 claims these dependencies needed to be Python 3 compatible first:

  • js.jquery (which is blocking js.bootstrap)
  • js.chosen
  • js.jquery_form (which is blocking js.deform)
  • js.jquery_maskedinput (which is blocking js.deform)
  • js.jquery_maskmoney (which is blocking js.deform)
  • js.jquery_sortable (which is blocking js.deform)
  • js.jquery_timepicker_addon (which is blocking js.deform)
  • js.jqueryui (which is blocking js.deform)
  • js.modernizr (which is blocking js.deform)
  • js.tinymce (which is blocking js.deform)
  • js.fineuploader
  • js.html5shiv
  • js.jquery_tablednd
  • js.jqueryui_tagit
  • kotti_tinymce
  • plone.scale
  • pytz
  • repoze.zcml (which is blocking repoze.workflow)
  • shutilwhich
  • yurl

Some (most?) of them are false positives, because they do officially support Python 3 and are just lacking the corresponding trove classifiers. All they js.* should work with no or very little modification. shutilwhich is a Python 2 backport of a module from the Python 3 standard library.

This might change until we start working on this ourselves, but even if it won't and we'd need to add Python 3 support to some dependencies ourselves, this seems absolutely doable.

@disko disko added this to the 1.x milestone Nov 22, 2014

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Apr 19, 2015

@disko I've just re-checked these, somewhat laboriously, since caniusepython3 seems to fail with a MemoryError when checking kotti_tinymce.
(Ran with caniusepython3 -r kotti-stable-requirements.txt and caniusepython3 -p kotti_tinymce)

The following still need porting/classifiers amending:

  • bleach-whitelist
  • js.jquery (which is blocking js.bootstrap)
  • js.chosen (blocked by js.jquery)
  • js.deform (blocked by all the other js packages)
  • js.jquery_form (which is blocking js.deform)
  • js.jquery_maskedinput (which is blocking js.deform)
  • js.jquery_maskmoney (which is blocking js.deform)
  • js.jquery_sortable (which is blocking js.deform)
  • js.jquery_timepicker_addon (which is blocking js.deform)
  • js.jqueryui (which is blocking js.deform)
  • js.modernizr (which is blocking js.deform)
  • js.tinymce (which is blocking js.deform)
  • js.fineuploader
  • js.html5shiv
  • kotti_tinymce (? MerroyError during check)
  • plone.scale (Blocked by Persistence)
  • shutilwhich
  • usersettings

bleach-whitelist looks be a trivial case/port/might just work.
plone.scale might work on Python3 with the pure python implementation of Persistence (?)

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Apr 19, 2015

The cause of the MemoryError described above is caniusepython3 doesn't handle recursive dependencies gracefully, and:
Kotti depends on kotti_tinymce and kotti_tincymce depends on Kotti.

@disko

This comment has been minimized.

Copy link
Member

disko commented Apr 20, 2015

This circular dependency is indeed something that really bugs me. Actually Kotti doesn't really depend on kotti_tinymce, it's just there because we want WYSIWYG editing out of the box. Maybe we should drop it from the package's dependencies but leave it in requires.txt and app.ini / development.ini?

Opinions? (/cc @dnouri, @tiberiuichim)

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Apr 20, 2015

@disko

This comment has been minimized.

Copy link
Member

disko commented Apr 20, 2015

bleach-whitelist is only a collection of strings, the js.* packages just call functions / methods from fanstatic and pass them strings. They all just need to use unicode literals instead of plain Python 2 strings and proper trove classifiers in their respective setup.py.

usersettings is probably the same – it's a very simple package (just ~100 LOC) that determines where to store user settings on various OSs. We use it to remember the last used answers to the questions asked during scaffolding. If it turned out not to be a simple fix, we could replace it with anything else or just drop it (at the sole cost of just losing some convenience).

shutilwhich is a backport of shutil.whichfrom the Python 3 standard library, so there's no need to port it.

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Apr 20, 2015

I'm sure there was/is a reason kotti_tinymce is maintained as a separate package -
Assuming its not used outside of Kotti, could the package be 'inlined' into the kotti core as a regular sub-package?

@disko

This comment has been minimized.

Copy link
Member

disko commented Apr 20, 2015

The idea is to let users chose which WYSIWYG editor to use (if any at all). Currently there's only kotti_tinymce but there could be others, and probably should – nowadays much leaner solutions exist.

I strongly oppose moving its code to the Kotti package.

@tiberiuichim

This comment has been minimized.

Copy link
Contributor

tiberiuichim commented Apr 20, 2015

If TinyMCE is just one of the possible choices, then I think we can remove it from Kotti's setup.py and place it in requirements.txt only.

Also, perhaps we can get rid of plone.scale as a dependency? The code is BSD and we're only using the scale.py module, a 160 lines module. Or maybe find some other generic package that does resizing based on Pillow.

@castaf

This comment has been minimized.

Copy link
Contributor

castaf commented Apr 20, 2015

+1

Le 20 avr. 2015 à 09:28, Andreas Kaiser notifications@github.com a écrit :

The idea is to let users chose which WYSIWYG editor to use (if any at all). Currently there's only kotti_tinymce but there could be others, and probably should – nowadays much leaner solutions exist.

I strongly oppose moving its code to the Kotti package.


Reply to this email directly or view it on GitHub #348 (comment).

@disko

This comment has been minimized.

Copy link
Member

disko commented Apr 20, 2015

If TinyMCE is just one of the possible choices, then I think we can remove it from Kotti's setup.py and place it in requirements.txt only.

It is just a choice, despite being the only one ATM.

Also, perhaps we can get rid of plone.scale as a dependency? The code is BSD and we're only using the scale.py module, a 160 lines module. Or maybe find some other generic package that does resizing based on Pillow.

w.r.t. Python 3 compatibility that might make sense. The code we actually use is even less: we only use the thumbnail resizing, which shouldn't be more than a few lines using only Pillow.

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Sep 29, 2015

I have a branch that's a work in progress... ~40 test failures.
https://github.com/mgrbyte/Kotti/commit/3990a9750a4eff9683bd0d32c43ec3e8f162c3c6

@disko

This comment has been minimized.

Copy link
Member

disko commented Sep 29, 2015

Wow, good job!

A few remarks and questions (partially also apply to your notfound branch):

  • yes, webtest is the way to go for new tests, so much less pain than the doctests.
  • what's the reason for using zope.interface.implementer instead of zope.interface.implements?
  • "setup.cfg: configparser does not like %.min.[css,js](kotti_tinymce, kotti)": FWIW, you can remove the [minify_*] sections as well as the minify alias and the minify in development_requires. For the few files we have, we can do minification manually – they hardly ever change at all.
@disko

This comment has been minimized.

Copy link
Member

disko commented Sep 29, 2015

I have a branch that's a work in progress... ~40 test failures.

Does that imply that you have Kotti running on Python 3 and "only" the tests are failing?

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Sep 29, 2015

Yep :)
pshell development.ini works
Need to test the UI, but it starts.
~40 test fail, most of the browser tests, some low hanging fruit (urllib imports, avoid using the unicode builtin)

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Sep 29, 2015

  • zope.interface.implementer vs zope.interface.implements
    • I think its the modern way to spell it
    • I prefer the syntax over the class body solution.
    • There's a deeper technical reason for my preference to do with the implementation, one day the use of the sys.getframe hack may be removed .
  • minify - good to know! this needs doing in kotti_tinymce too
@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Sep 29, 2015

@disko Does Kotti need to support Python 2.6 going forward?
It would make this much easier if it didn't.

I think converting the doc-tests to webtest tests would be good thing to do.
Had I done that first, I think it might have make this porting exercise easier, not sure about doing that now since I'd like to get this "done" before any potential major change comes in (avoid merge hell)

@disko

This comment has been minimized.

Copy link
Member

disko commented Sep 29, 2015

@disko

This comment has been minimized.

Copy link
Member

disko commented Sep 29, 2015

On 29 Sep 2015, at 11:35, Matt Russell wrote:

  • minify - good to know! this needs doing in kotti_tinymce too

Done:
Kotti/kotti_tinymce@3f99922.

It's not used there anymore. The whole static resource management and
bundling was migrated to npm, bower and gulp.

@disko

This comment has been minimized.

Copy link
Member

disko commented Sep 29, 2015

@tiberiuichim

This comment has been minimized.

Copy link
Contributor

tiberiuichim commented Sep 29, 2015

No need for python 2.6 on my side

@mgrbyte

This comment has been minimized.

Copy link
Contributor

mgrbyte commented Sep 29, 2015

Down to 1 real failing test now.
7 others, which are browser*.txt tests, or those that otherwise depend on wsgi_intercept.zope_testbrowser (TestUploadFile)

  • TestNode.test_root_acl, Ordering of lists is different, kotti.tests.test_node:15

I've tested in the UI, and everything seems to work (login, logout, add user, saving, sharing et al)

0a797cc

Tentative unicode test fix (this needs more eyes especially)
6349625

@dnouri

This comment has been minimized.

Copy link
Member

dnouri commented Sep 29, 2015

I'm also fine with dropping Python 2.6 support. Awesome work, by the way!

@disko

This comment has been minimized.

Copy link
Member

disko commented Sep 30, 2015

So let's do it: drop Python 2.6 and let's have Python 3 instead.

@disko

This comment has been minimized.

Copy link
Member

disko commented Feb 22, 2018

DONE :)

@disko disko closed this Feb 22, 2018

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