Page MenuHomeFreeBSD

graphics/py-traitsui: Update to 6.1.2 and add options for extra dependencies
ClosedPublic

Authored by kai on Aug 12 2019, 3:42 PM.

Details

Summary
graphics/py-traitsui: Update to 6.1.2 and add options for extra dependencies

* Update to 6.1.2 [1]

While I'm here:

* Add non-default options for the Python extra dependencies
* Hide the WXGTK option for Python 3 as the wx tookit isn't available for Python3, yet. (uses a similar approach as in rP503386 for www/py-autobahn)

Changelog:

  https://github.com/enthought/traitsui/blob/6.1.1/CHANGES.txt

PR: 238469
Submitted by: vladimir.chukharev@gmail.com (maintainer) [1]
Reviewed_by: koobs (python)
Differential_Revision: D21229
Test Plan
  • poudriere -> OK (11.2-, 11.3-, 12.0-RELEASE, 13.0-CURRENT@r350665 amd64 + i386) for each py27 + py36 flavor
  • portlint -> Gives some warnings about the rather unusual approach in the Makefile
  • When using Python 3 the WXGTK option doesn't appear with "make config"

The dependencies for this port are obtained through traitsui/__init__.py via setup.py:

`
requires = [

'traits',
'pyface>=6.0.0',
'six',

]
extras_require = {

'wx': ['wxpython>=2.8.10', 'numpy'],
'pyqt': ['pyqt>=4.10', 'pygments'],
'pyqt5': ['pyqt>=5', 'pygments'],
'pyside': ['pyside>=1.2', 'pygments'],
'demo': ['configobj'],

`

I've omitted "pyqt" (= Qt4 is EOL and gone) and "pyside" (= not in ports) as options from the Makefile.

Questions that originates from the related PR:

  • Is there a better usage of the option framework to reflect the extra dependencies regarding the issue that WXGTK is only available for Python 2?
  • Is it possible to reduce the RUN_DEPENDS as devel/py-pyface has already them?

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kai created this revision.Aug 12 2019, 3:42 PM

It seems to me that options can be made more user friendly. Or completely abandoned for now. May be, until pyside is added.
In particular, for py36, the only possible toolkit is QT5, and it comes off by default. Logically, it can be unconditionally used in this flavor.
For py27, there is QT5 and WXGTK. None is selected by default, and I think, at least one is compulsory. Do they both work with traitsui? Or can we choose the better of the two? (I still cannot build it for py27, some deps fail to install with their py36 flavors installed.)
The DEMO option adds some dependencies. Is this useful without demos from examples directory? I cannot get any demo actually show anything, even with the examples...

kai updated this revision to Diff 60826.Aug 15 2019, 1:04 PM

Addressing Vladimir's suggestions.

kai added a comment.Aug 15 2019, 1:06 PM

It seems to me that options can be made more user friendly. Or completely abandoned for now. May be, until pyside is added.
In particular, for py36, the only possible toolkit is QT5, and it comes off by default. Logically, it can be unconditionally used in this flavor.
For py27, there is QT5 and WXGTK. None is selected by default, and I think, at least one is compulsory. Do they both work with traitsui? Or can we choose the better of the two? (I still cannot build it for py27, some deps fail to install with their py36 flavors installed.)
The DEMO option adds some dependencies. Is this useful without demos from examples directory? I cannot get any demo actually show anything, even with the examples...

Thank you for the suggestions and hints. I removed the DEMO and WXGTK option and the fdiff should now a bit more cleaner. I also moved the comment about the dependencies under RUN_DEPENDS to reduce the warnings from portlint.

As for the dependencies for the DEMO option: There's only one script (= traitsui/extras/demo.py) that make of use of devel/py-configobj. According to your info that any demo won't run it might be safe to leave that DEMO option completely out.

Because the Python support for the wx toolset is still incomplete at the moment, maybe it makes more sense to leave that option out for now and only add an comment similar as in devel/py-robotframework-ride so that this can revised in future?

But I'm also completely fine if we leave the whole options out for now. In that case it would make sense to add a small comment in the port's Makefile about the optional extra dependencies.

koobs added a subscriber: koobs.EditedAug 16 2019, 2:57 AM

The FreeBSD Python ports policy has recently been updated to clarify USES=python guidelines.

Accordingly, USES=python here doesn't declare a <version-spec>, and setup.py documents the following for Python support:

Programming Language :: Python :: 2.6
Programming Language :: Python :: 2.7
Programming Language :: Python :: 3.4
Programming Language :: Python :: 3.5

Port should explicitly be tested with Python > 3.5 (ideally with its test suite) to confirm support for those versions, otherwise the USES=python:<version-spec> should be capped at 3.5

If the port supports versions > 3.5, Trove Classifiers should be updated to reflect current reality (perhaps theyre already CI'ing versions > 3.5), send an upstream PR or open an issue to update it

koobs added a comment.Aug 16 2019, 3:00 AM

Also, tests are available upstream (but not in the sdist), but it would be great to be able to QA this complex port properly.

See: https://docs.enthought.com/traitsui/#running-the-test-suite

Ask upstream to include all files/data files required for testing in the sdist (via MANIFEST.in modification)

In the meantime, one may use USE_GITHUB to grab the sources until the sdist ships tests/test files

koobs edited the summary of this revision. (Show Details)Aug 16 2019, 11:17 AM
koobs edited the summary of this revision. (Show Details)
kai updated this revision to Diff 60910.Aug 16 2019, 8:00 PM

Adressing @koobs's suggestions:

  • Adding a test target with the required dependencies to make QA easier. (Tested with py27 and py36 -> OK)

During the tests it came to light that a few more Qt5 dependencies were required to get successful test results.

kai added a comment.Aug 16 2019, 8:17 PM

Also, tests are available upstream (but not in the sdist), but it would be great to be able to QA this complex port properly.
See: https://docs.enthought.com/traitsui/#running-the-test-suite
Ask upstream to include all files/data files required for testing in the sdist (via MANIFEST.in modification)
In the meantime, one may use USE_GITHUB to grab the sources until the sdist ships tests/test files

Thank you for the hints! The etstool.py was the key to get usable test-targets for QA. Porting etstool.py and EDM (= Enthought Deploy Manager) doesn't make much sense as far I can see. Both seem to work like a wrapper for devel/py-pip and makes heavy use of it.

Thus I extracted the needed dependencies and the tests commands from etstool.py and the tests looked fine so far (with/without Qt5 for py27 + py36).

The etstool.py is only supplied with the GitHub sources but the required tests are already available in the sdist (= traitsui/tests + traitsui/qt4/tests).

I tested the new version on my system. There are still some minor issues.

  1. From portlint:
$ portlint -Ca
WARN: Makefile: When USES=pyqt:5 is defined, you must also define USE_PYQT=xxxx
0 fatal errors and 1 warning found.
  1. The option QT5 is on by default, and, if I understand correctly, while it is the only one, it must be selected. So, what's reason to have it? Comment would be sufficient IMHO.
  2. One test fails.
$ sudo make test
....
FAIL: test_wrap_text_narrow_short (traitsui.qt4.tests.test_helper.TestWrapText)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/traitsui/qt4/tests/test_helper.py", line 105, in test_wrap_text_narrow_short
    self.assertEqual(lines[19][-1], '\u2026')
AssertionError: '.' != '\u2026'
- .
+ \u2026

....

I think all these are really minor things and the patch can be accepted as is. Thanks so much indeed!

PS. Sorry for the delay with my reaction.
PPS. I plan to clarify with the developer what might be the reason of the test failure. That often is a long process, from my experience, so no wait right now.

I discussed the tests failure with developers. The source of it is that Qt in FreeBSD uses three dots instead of the unicode ellipsis symbol (supposedly, elidedText() function).
The test has been changed to admit "..." as ellipsis. This will come with the next release.
We can simply ignore the test failure for now. I plan to find the exact place where ellipsis is inserted, and to try using the unicode ellipsis symbol (but don't hold your breath).

koobs added a comment.Aug 27 2019, 4:37 AM

I discussed the tests failure with developers. The source of it is that Qt in FreeBSD uses three dots instead of the unicode ellipsis symbol (supposedly, elidedText() function).
The test has been changed to admit "..." as ellipsis. This will come with the next release.
We can simply ignore the test failure for now. I plan to find the exact place where ellipsis is inserted, and to try using the unicode ellipsis symbol (but don't hold your breath).

Nice. You should be able to exclude specific tests with nose's -e REGEX, --exclude=REGEX argument

kai updated this revision to Diff 61339.Aug 27 2019, 7:52 AM
  • Remove non-compulsory test requirements (= coverage) and update the test targets accordingly.
  • Exclude the test that fail with option QT5 enabled for a while
  • Add/update comments
  • Make the QT5 option as default as optional dependencies should be enabled according to the Python policy.
koobs accepted this revision.Aug 27 2019, 8:15 AM
koobs edited the summary of this revision. (Show Details)

LGTM if it passes QA (portlint, poudriere (py 2/3), make test)

This revision is now accepted and ready to land.Aug 27 2019, 8:16 AM
kai added a comment.Aug 27 2019, 9:05 AM

I tested the new version on my system. There are still some minor issues.

  1. From portlint:
$ portlint -Ca
WARN: Makefile: When USES=pyqt:5 is defined, you must also define USE_PYQT=xxxx
0 fatal errors and 1 warning found.

This warning can be ignored as portlint can't find the USE_PYQT variable that is "hidden" in the QT5_USE=PYTQ=[...] statement.

  1. The option QT5 is on by default, and, if I understand correctly, while it is the only one, it must be selected. So, what's reason to have it? Comment would be sufficient IMHO.

The QT5 option wasn't on by default in the previous diffs because I left it intentionally out to keep things more flexible during the rest of the course. I enabled the QT5 option now as default one and according the Python ports policy extra dependencies should be defined as options. This will also simplify adding/removing options (e.g. for WX once wxWidgets 4.x is available in the Ports tree) in the future.

  1. One test fails.

Good catch and thank you for working with upstream to clarify this! I excluded that test from the test target for the QT5 option for a while.

PS. Sorry for the delay with my reaction.

Nothing to worry, good things need time.

This revision was automatically updated to reflect the committed changes.