Skip to content
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

Python 3 Compatibility #209

Merged
merged 65 commits into from
May 17, 2016
Merged

Python 3 Compatibility #209

merged 65 commits into from
May 17, 2016

Conversation

corranwebster
Copy link
Contributor

Updated version of #137

Pankaj Pandey and others added 27 commits October 24, 2013 03:22
Simple str-bytes/int-long conversions, remove type string
functions from numeric.i absent in python3
- types.ListType -> list ...
- migrate swig typemaps
Test was expecting ValueError, but an AssertionError was being
raised.
Also added kiva/tests/__init__.py so tat tests can be run from
installation.
All unit tests now pass on python 3.3 and 2.7 on linux.
Few integration tests segfault, as was the case before.
NOTE: The test file currently segfaults with gcc (4.8.2) possibly
due to compiler bug on optimization level >= O1;
use clang instead to compile kiva
Somehow, 2to3 is not always able to detect it, ,likely due to
agg being an extension module.
…ature/python3

Conflicts:
	.travis.yml
	enable/__init__.py
	integrationtests/kiva/agg/test_draw_dash.py
	kiva/fonttools/font.py
	kiva/fonttools/sstruct.py
	setup.py
@corranwebster
Copy link
Contributor Author

Looks like kiwisolver isn't Python 3 compatible?

@corranwebster
Copy link
Contributor Author

Yep, Kiwi solver doesn't support Python 3 yet, and probably won't soon. I'm guessing we can skip the constraints-based layout tests in the short-term, but the lack of the feature for Python 3 is a bit of a disappointment.

@jwiggins Thoughts on ways forward?

@itziakos
Copy link
Member

@jwiggins and @corranwebster even with python3 support for kiwisolver we still have the problem that the kiwisolver depedency is not optionional.

@corranwebster
Copy link
Contributor Author

@jwiggins and @itziakos Would it make sense to vendorize Kiwisolver inside enable.layout.*

@itziakos
Copy link
Member

@jwiggins and @itziakos Would it make sense to vendorize Kiwisolver inside enable.layout.*

I think that vendorizing kiwisolver to solve the non-optional relationship with enable is wrong. Kiwisolver and the constrained based layout should be optional.

@jwiggins
Copy link
Member

@itziakos: I agree that constraint-based layout should be optional.

I disagree that such an arrangement should be implemented here. This PR is for Python 3.

@itziakos
Copy link
Member

I disagree that such an arrangement should be implemented here. This PR is for Python 3.

This is fine with me. I have created a new issue #212 for the task of making kiwisolver truly optional. However, please note that at the moment due to the fact that kiwisolver is not python 3 compatible, the resolution of #212 or nucleic/kiwi#13 are blocking this PR.

@jwiggins
Copy link
Member

jwiggins commented Mar 9, 2016

nucleic/kiwi#13 has now been merged! This PR can move forward.

@corranwebster
Copy link
Contributor Author

corranwebster commented May 17, 2016

Everything is passing. The native scrollbar tests are passing, but a bit dodgy---but then the native scrollbar is a bit dodgy. I'll open a ticket for that, I don't think it's because this PR is bad Update: it looks like this is a regression introduced by this PR. Will look more closely before we merge.

@jwiggins @itziakos Any show-stoppers that would prevent merging this?

@itziakos
Copy link
Member

itziakos commented May 17, 2016

@corranwebster in Qt you need to run the event loop in order to destroy widgets, using process events will not work as expected (probably wait for the widget to be destroyed https://github.com/enthought/pyface/blob/master/pyface/ui/qt4/util/gui_test_assistant.py#L79)

@codecov-io
Copy link

Current coverage is 49.36%

Merging #209 into master will increase coverage by 14.01%

@@             master       #209   diff @@
==========================================
  Files           211        123     -88   
  Lines         18659      12828   -5831   
  Methods           0          0           
  Messages          0          0           
  Branches       2621       2045    -576   
==========================================
- Hits           6596       6332    -264   
+ Misses        11543       5936   -5607   
- Partials        520        560     +40   
  1. 2 files in kiva/fonttools were modified. more
    • Misses +13
    • Hits +14
  2. 3 files in kiva were modified. more
    • Misses +8
    • Partials +2
    • Hits -10
  3. 11 files in enable were modified. more
    • Misses +195
    • Partials -8
    • Hits -73
  4. 2 files (not in diff) in kiva/fonttools were deleted. more
  5. 2 files (not in diff) in kiva were deleted. more
  6. 2 files (not in diff) in ...ble/trait_defs/ui/wx were deleted. more
  7. 3 files (not in diff) in enable/trait_defs/ui were deleted. more
  8. 1 files (not in diff) in enable/trait_defs were deleted. more
  9. 2 files (not in diff) in ...nable/tools/toolbars were deleted. more
  10. 6 files (not in diff) in enable/tools were deleted. more

Powered by Codecov. Last updated by 29a676d...7678b91

@corranwebster
Copy link
Contributor Author

corranwebster commented May 17, 2016

There appears to be a Heisenbug in the viewport_test_case.py test cases under Python 3. Simply re-running can get the tests to pass. I can occasionally get a replication locally when running in Python 3.

@itziakos
Copy link
Member

There appears to be a Heisenbug in the viewport_test_case.py test cases under Python 3. Simply re-running can get the tests to pass. I can occasionally get a replication locally when running in Python 3.

It is possible that a number of object deletions and other events are leaking between tests and that can result in unsuspected behavior and timeouts down the line. I would open an issue about the random failure. We need to cleanup the testing infra and make sure that tests do not leak widgets in the Qt ar Wx application singletons

@itziakos
Copy link
Member

@corranwebster do you still want to keep the workaround on making kiwisolver optional (see coordinate_box)? As far as I understand now that kiwisolver supports python3 we can delegate making kiwisolver optional at a later time.

@corranwebster
Copy link
Contributor Author

It's replicable just running that test case alone; so its not leakage from other tests; although it could be leakage from within the one test.

@corranwebster
Copy link
Contributor Author

I'm OK with making it optional being in here even if kiwisolver now supports Python 3. We can revisit later if needed.

@jwiggins
Copy link
Member

@corranwebster: 👍 from my end

@corranwebster
Copy link
Contributor Author

Looks like it is an initialisation order issue for the Heisenbug: the order in which traits get set at start-up time seems variable under Python 3, and if you get the wrong order then you can end up with bad results.

I will merge this, and open an issue for the bug, as it's not a 5 minute fix, by the look of it.

@corranwebster
Copy link
Contributor Author

See #228 for issue.

@corranwebster corranwebster merged commit d5720c0 into master May 17, 2016
@corranwebster corranwebster deleted the feature/python3 branch May 17, 2016 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants