Unicode trait handling for those vtk methods returning unicode data #355

Merged
merged 8 commits into from Apr 27, 2016

Conversation

Projects
None yet
4 participants
@stefanoborini
Contributor

stefanoborini commented Apr 25, 2016

The generator code did not handle unicode, but some vtk routines do return them. Although never able to reproduce it, issue #354 shows a situation where it does matter.

I am unsure about potential side effects of this change on client code, but I am surprised it worked until now, and I expect the method was simply not generated, or it was but it returned an empty string.

stefanoborini added some commits Apr 25, 2016

Unicode conversion of return value
First commit, needs well formed test and check on platform
tvtk/wrapper_gen.py
+ if default == '\x00':
+ t_def = 'traits.Unicode(u"", '
+ elif default == '"':
+ t_def = "traits.Unicode(u'%(default)s', " % locals()

This comment has been minimized.

@rkern

rkern Apr 25, 2016

Member
if default == u'\x00':
    default = u''
t_def = 'traits.Unicode({0!r}, enter_set=True, auto_set=False)'.format(default)
@rkern

rkern Apr 25, 2016

Member
if default == u'\x00':
    default = u''
t_def = 'traits.Unicode({0!r}, enter_set=True, auto_set=False)'.format(default)

This comment has been minimized.

@rkern

rkern Apr 25, 2016

Member

Don't .encode() or clean_special_chars(). {!r} takes care of everything.

@rkern

rkern Apr 25, 2016

Member

Don't .encode() or clean_special_chars(). {!r} takes care of everything.

This comment has been minimized.

@stefanoborini

stefanoborini Apr 25, 2016

Contributor

@rkern done. I also took care of the part handling String and removed the cleaning function.

@stefanoborini

stefanoborini Apr 25, 2016

Contributor

@rkern done. I also took care of the part handling String and removed the cleaning function.

stefanoborini added some commits Apr 25, 2016

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi Apr 25, 2016

Member

Verified that it works with the following tox.ini

[tox]
envlist = py27

[testenv]
# To make sure we actually test the tarball and now the working directory
changedir = .tox

# We use symbolic links to provide vtk which is not on pypi
whitelist_externals=/bin/ln

# Install development requirements
deps = numpy

# e.g. export VTK_PATH=${HOME}/.cache/VTK-6.3.0-Linux-64bit/lib/python2.7/site-packages/vtk
# e.g. export LD_LIBRARY_PATH=${HOME}/.cache/VTK-6.3.0-Linux-64bit/lib
passenv = VTK_PATH LD_LIBRARY_PATH

basepython = python2.7

commands =
     ln -sf {env:VTK_PATH} {envsitepackagesdir}/
Member

kitchoi commented Apr 25, 2016

Verified that it works with the following tox.ini

[tox]
envlist = py27

[testenv]
# To make sure we actually test the tarball and now the working directory
changedir = .tox

# We use symbolic links to provide vtk which is not on pypi
whitelist_externals=/bin/ln

# Install development requirements
deps = numpy

# e.g. export VTK_PATH=${HOME}/.cache/VTK-6.3.0-Linux-64bit/lib/python2.7/site-packages/vtk
# e.g. export LD_LIBRARY_PATH=${HOME}/.cache/VTK-6.3.0-Linux-64bit/lib
passenv = VTK_PATH LD_LIBRARY_PATH

basepython = python2.7

commands =
     ln -sf {env:VTK_PATH} {envsitepackagesdir}/
@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini Apr 25, 2016

Contributor

Failure seems due to a network problem. Restarting

Contributor

stefanoborini commented Apr 25, 2016

Failure seems due to a network problem. Restarting

tvtk/wrapper_gen.py
+
+ elif PY_VER < 3 and typ is unicode:
+ if default == u'\x00':
+ default = ''

This comment has been minimized.

@rkern

rkern Apr 26, 2016

Member

default = u''

@rkern

rkern Apr 26, 2016

Member

default = u''

@prabhuramachandran prabhuramachandran merged commit e633f8f into master Apr 27, 2016

4 of 7 checks passed

codecov/changes --- - No report found to compare against.
Details
codecov/patch --- - No report found to compare against.
Details
codecov/project --- - No report found to compare against.
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@prabhuramachandran prabhuramachandran deleted the unicode-coercion branch Apr 27, 2016

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