Add examples #119

fredrikaverpil · 2016-08-29T12:46:02Z

Docker for Mac is acting up here and I can't perform tests locally... so I'm going to have to rely on Travis. Bare (bear?) with me for a while...

fredrikaverpil · 2016-08-29T13:05:46Z

Hm, not sure why you can't see the tests in Travis. This is what I see locally:

$ docker run --rm -v $(pwd):/Qt.py mottosso/qt.py27

Doctest: test_caveats.test_1_qtgui_qabstractitemmodel_createindex ... ok
Doctest: test_caveats.test_2_qtgui_qabstractitemmodel_createindex ... ok
Doctest: test_caveats.test_3_qtcore_qitemselection ... ok
Doctest: test_caveats.test_4_qtcore_qitemselection ... ok
Doctest: test_caveats.test_5_qtcore_qitemselection ... ok
Doctest: test_caveats.test_6_qtcore_qitemselection ... ok
Doctest: test_caveats.test_7_qtcore_slot ... ok
Doctest: test_caveats.test_8_qtcore_slot ... ok
Doctest: test_caveats.test_9_qtwidgets_qaction_triggered ... ok
Doctest: test_caveats.test_10_qtwidgets_qaction_triggered ... ok
Doctest: test_caveats.test_11_qtwidgets_qheaderview_setresizemode ... ok
Doctest: test_caveats.test_12_qtwidgets_qheaderview_setresizemode ... ok
Doctest: test_caveats.test_13_qtwidgets_qheaderview_setresizemode ... ok
Doctest: test_examples.test_1_base_instance_as_argument_using_getattr_setattr ... ok
Tests require all bindings to be installed ... ok
Setting QT_PREFERRED_BINDING to PyQt4 properly forces the binding ... ok
Setting QT_PREFERRED_BINDING to PyQt5 properly forces the binding ... ok
Setting QT_PREFERRED_BINDING to PySide properly forces the binding ... ok
Setting QT_PREFERRED_BINDING to PySide2 properly forces the binding ... ok
Setting QT_PREFERRED_BINDING to more than one binding excludes others ... ok
Preferring None shouldn't import anything ... ok
Qt.py may be use alongside the actual binding ... ok
Preferred binding PyQt4 should have sip version 2 ... ok
Qt.py may be bundled along with another library/project ... ok
Raise ImportError if sip API v1 was already set (Python 2.x only) ... ok

----------------------------------------------------------------------
Ran 25 tests in 6.910s

OK

Note this:

Doctest: test_examples.test_1_base_instance_as_argument_using_getattr_setattr ... ok

So, what I did was:

build_caveats_tests.py -> build_md_tests.py (md = Markdown)
caveats.py -> mdparse.py

Then I added an /examples folder with a load_ui.md example in it which gets parsed, just like CAVEATS.md. I extended build_md_tests.py to cover any example in /examples.

What do you think about this approach?

I also noticed the sleep 3 command in the Dockerfiles wasn't enough for my Mac laptop. I increased that to 5 seconds.

This reverts commit 3d6cdb0.

mottosso · 2016-08-29T13:20:34Z

Hey,

First off, great idea to make examples runnable. I'm not entirely keen on the idea of examples having the >>> prefixes, as it makes it difficult to copy/paste to run them..

Would it be possible to make them simple Python files, without the need for parsing?

fredrikaverpil · 2016-08-29T13:29:25Z

Yes, I agree. I think that would make it easier for anyone to contribute as well. I'll try to come up with something which can be included in the tests.

I also renamed the builders, because they were getting run during testing due to their name containing the magic word "test".

fredrikaverpil · 2016-08-29T14:08:52Z

Hm...

$ docker run --rm -v $(pwd):/Qt.py mottosso/qt.py27

Doctest: test_caveats.test_1_qtgui_qabstractitemmodel_createindex ... ok
Doctest: test_caveats.test_2_qtgui_qabstractitemmodel_createindex ... ok
Doctest: test_caveats.test_3_qtcore_qitemselection ... ok
Doctest: test_caveats.test_4_qtcore_qitemselection ... ok
Doctest: test_caveats.test_5_qtcore_qitemselection ... ok
Doctest: test_caveats.test_6_qtcore_qitemselection ... ok
Doctest: test_caveats.test_7_qtcore_slot ... ok
Doctest: test_caveats.test_8_qtcore_slot ... ok
Doctest: test_caveats.test_9_qtwidgets_qaction_triggered ... ok
Doctest: test_caveats.test_10_qtwidgets_qaction_triggered ... ok
Doctest: test_caveats.test_11_qtwidgets_qheaderview_setresizemode ... ok
Doctest: test_caveats.test_12_qtwidgets_qheaderview_setresizemode ... ok
Doctest: test_caveats.test_13_qtwidgets_qheaderview_setresizemode ... ok
Segmentation fault

# Conflicts: # Qt.py

Without these lines, tests may look within any Python file and throw errors unrelated to tests (in this case, SyntaxError in Python 3 for file(s) produced by build_membership.py).

mottosso · 2016-08-29T16:00:28Z

This will take you into the image so you can test interactively.

$ docker run -ti --rm --entrypoint bash -v $(pwd):/Qt.py mottosso/qt.py27
$ cd /Qt.py
$ python build_examples_tests.py
$ python some_test.py..

fredrikaverpil · 2016-08-29T16:34:16Z

What do you think so far?

fredrikaverpil · 2016-08-29T16:37:53Z

Hm. I miss having an .md file with explanations.

mottosso · 2016-08-29T16:42:43Z

examples/load_ui_base_instance.py

+import sys
+import os
+
+os.environ["QT_PREFERRED_BINDING"] = "PySide"


How come you prefer PySide here? Shouldn't this work on any binding?

I get a weird thing in Python 2.7 if I don't specify this. Don't really understand why. I can remove it and you'll see in my next commit.

Ok. If unspecified, it will try and use PySide2. Maybe there's some issue there.

fredrikaverpil · 2016-08-30T16:37:35Z

@mottosso what do you think?

mottosso · 2016-08-30T16:47:18Z

examples/load_ui/load_ui_1.py

+import os
+
+# Set preferred binding
+os.environ["QT_PREFERRED_BINDING"] = "PySide:PyQt4"


Could you make this os.pathsep.join(["PySide", "PyQt4"]) so that it also runs on Windows?

(I would also be up for discussing an alternative syntax for this, having now seen it in the real-world, e.g. "PySide,PyQt4")

mottosso · 2016-08-30T16:51:49Z

I like it!

fredrikaverpil · 2016-08-30T16:55:20Z

Great points, I agree on all of them. My eyes hurt as well now ;)

I'll address them all, probably tomorrow.

fredrikaverpil · 2016-08-31T07:19:38Z

Fixed:

Using os.pathsep.join
Docstrings (google style)
snake_case (no more camelCase)
Only using " in docstrings and ' for everything else

I'm also copying non-python files into the working directory, so that the ui file can be defined without the relative filepath into /examples/etc.../file.ui. But I have another suggestion which I'll write about in a minute...

(I would also be up for discussing an alternative syntax for this, having now seen it in the real-world, e.g. "PySide,PyQt4")

Yes, I think a comma is more suitable. I always thought : or ; were indicating a filesystem path (which in this case we don't want to imply). The comma symbol is suitable to indicate that there's a specific order of things. The first item in the comma separated list is first used etc...

fredrikaverpil · 2016-08-31T07:23:01Z

So, I'm copying files from /examples/*/*.* into the working directory. This is not so nice, as we might end up with filename clashes.

If I put an __init__.py inside of the examples folder and also inside of each example folder, I can add use --exe and nose will parse all example files prepended with test. Or I could define --exe examples/*/*.py and do without the __init__.py files. Or I can combine the two to avoid prepending filenames but enable importability.

This will also eliminate the need for build_examples.py.

If we use the __init__.py approach, this creates a module (or "library" if you will) of examples inside of Qt.py. I'm not saying we are to bundle that with Qt.py via PyPi but we could if we decide to do so in the future. Then these example functions will become available by doing something like:

from examples.load_ui import load_ui_1

load_ui_1.test()  # Run the test function
widget = load_ui_1.setup_ui('qwidget.ui', self)  # Load .ui

(we'd need to rename those python files to avoid the ugly naming though)

What's nice about this is we've already talked about restructuring Qt.py into subfolders so that it'll be easier to maintain as it grows (but we agreed that is not the way forward). So instead of making Qt.py grow, /example can grow instead.

I'll push an example of this and we can discuss.

fredrikaverpil · 2016-08-31T07:39:21Z

Edited my previous post so much if you already read it I'm afraid I'd like you to re-read it. Sorry - made more sense to consolidate, I thought. 😢

fredrikaverpil · 2016-08-31T07:50:09Z

In the previous commit:

No need to build examples, build_examples.py removed.
Examples are being executed where they already reside, if they include a function prefixed with test since the --exe option now takes examples/*/*.py as value.
I added __init__.py to /examples and examples/load_ui which makes these examples importable.

from examples.load_ui import baseinstance1

widget = baseinstance1.setup_ui('examples/load_ui/qwidget.ui', self)

One might argue that with the importable examples, we should define our tests in test.py instead. But then the example itself won't be complete with example usage. So I think this approach is the nicest one for end-users (who are now not required to dive into the test.py to fully understand the example implementaiton).

I want to stress that I don't think that examples should in any way be distributed with Qt.py. It's bound to become a problem for us at some point if we do that and users start relying on its existance/structure/functionality.

@mottosso what do you think about all this?

mottosso · 2016-08-31T08:25:27Z

Yes, I think a comma is more suitable.

I know you did, and having worked with it for a while now, I think your gut was right.

If I put an init.py inside of the examples folder and also inside of each example folder,

This sounds good to me.

For the record, the --exe option isn't related to the next argument, it's a standalone flag meaning "run executable files, as well as non-executable files".

I'm not saying we are to bundle that with Qt.py via PyPi but we could if we decide to do so in the future.

We still could not, I think, as Qt.py and the examples/ directory would end up directly in site-packages/, making examples a globally available Python package; e.g. import examples.

It doesn't hurt having a package in this way in the project however, as we explicitly specify that the file Qt.py is the only file to be included in the PyPI package.

So instead of making Qt.py grow, /example can grow instead.

Conceptually, I like the sound of that, but am having trouble seeing how growth of any kind is helpful for such simple and de-coupled things as examples. I'll keep an open mind though.

One might argue that with the importable examples, we should define our tests in test.py instead.

Do you mean in the global github.com/Qt.py/tests.py, or in a local test.py per example?

fredrikaverpil · 2016-08-31T08:30:57Z

Do you mean in the global github.com/Qt.py/tests.py, or in a local test.py per example?

I meant that one might argue to collect all tests into github.com/Qt.py/tests.py but in this case I think that, for completion purposes, it's nicer to define test in each example. Also, defining the tests in the global tests.py would decouple the test and the example a bit too much, I think. So I like this current approach that is in this PR.

mottosso · 2016-08-31T08:41:53Z

Dockerfile-py2.7

 	Xvfb :99 -screen 0 1024x768x16 2>/dev/null & \
-	sleep 3 && \
+	sleep 10 && \


Woah, seems like this one has no end.

Let's find a better way of handling this. There must be some way of waiting for a signal/sign of sorts, and continuing once that happens?

Yes, I don't know if it's a memleak or what is going on... but I need to rebuild my containers at least once a day. And I have to restart XQuartz a few times too. So I'm not sure what's causing that.

But so 3 seconds isn't always enough... and that's why I increased it to something I don't have to go back and tweak with +1 second...

XQuartz

You shouldn't need anything like that on your machine; xvfb is completely isolated within the container.

I'm giving this a go now...

xdpyinfo -display :99 >/dev/null 2>&1 && echo "In use" || echo "Free"

Ah, this was a better/simpler approach:

ps auxx | grep Xvfb

I'm not following, but glad to hear you may have found a way.

I'm waiting until I find the process Xvfb to be running. Works here on Docker for Mac.

XQuartz

You shouldn't need anything like that on your machine; xvfb is completely isolated within the container.

Ah!

fredrikaverpil · 2016-08-31T09:52:54Z

So, now there's a loop waiting for the Xvfb process. Let me know what you think about this and the rest of this PR.

I think this could be ready for a merge.

mottosso · 2016-08-31T09:55:08Z

Dockerfile-py2.7

 	Xvfb :99 -screen 0 1024x768x16 2>/dev/null & \
-	sleep 3 && \
+    while ! ps aux | grep -q '[0]:00 Xvfb :99 -screen 0 1024x768x16'; do echo "Waiting for Xvfb..."; sleep 1; done && \


Neat!

Without the 2>/dev/null, are we getting output from xvfb at this point?

The new command just checks whether grep finds this string from the output of the ps aux command:

0:00 Xvfb :99 -screen 0 1024x768x16

The original command which launches Xvfb is untouched:

Xvfb :99 -screen 0 1024x768x16 2>/dev/null

So the functionality should be unchanged.

Aah, right, missed that, sorry.

That's great!

I could make the search string include that last pipe bit. I didn't do it just to shorten down the line length.

Is it possible to break this line?

... Xvfb :99 -screen 0 1024x768x16 2>/dev/null & \ while ! ps aux | grep -q '[0]:00 Xvfb :99 -screen 0 1024x768x16'; \ do echo "Waiting for Xvfb..."; sleep 1; done && \ nosetests \ ...

I could make the search string include that last pipe bit. I didn't do it just to shorten down the line length.

On second thought, I can't. ps doesn't show piping:

root 6 0.7 1.5 207336 31256 ? Sl 10:05 0:00 Xvfb :99 -screen 0 1024x768x16

So this is fine.

Is it possible to break this line?

Yes, done!

mottosso · 2016-08-31T10:45:41Z

LGTM!

fredrikaverpil · 2016-08-31T10:48:39Z

Awesome, I'll merge this then!

fredrikaverpil added 3 commits August 29, 2016 14:44

Add examples

2b93dbe

Increase sleep to 5 secs

6765130

Fix parse error

12bd3d1

fredrikaverpil and others added 3 commits August 29, 2016 15:07

Add headline

3d6cdb0

Revert "Add headline"

8fff5b9

This reverts commit 3d6cdb0.

Implement #105

b13dfcd

mottosso added 2 commits August 29, 2016 14:23

Implement #105

8f95c5d

Fix Dockerfile for Python 3.5

1fe8bda

mottosso and others added 7 commits August 29, 2016 15:00

Do not run under Python 3

b591fd5

I also renamed the builders, because they were getting run during testing due to their name containing the magic word "test".

Use standalone python scripts

0d69db9

Remove debug command

2985cec

Add examples testing to Python 3

fd5a9aa

Revert change

dd221c8

Revert change

fb2fbfa

Add newline

4ccc85e

mottosso added 3 commits August 29, 2016 15:33

Add missing members, exclude PySide2-only members

7474ce1

Merge remote-tracking branch 'origin/master' into #105

6264a44

# Conflicts: # Qt.py

Explicitly run test files

0003969

Without these lines, tests may look within any Python file and throw errors unrelated to tests (in this case, SyntaxError in Python 3 for file(s) produced by build_membership.py).

fredrikaverpil added 3 commits August 29, 2016 18:08

Attempt fix sefault

8885178

Remove file

ce09ec5

Set preferred binding

f643d01

mottosso reviewed Aug 29, 2016
View reviewed changes

fredrikaverpil mentioned this pull request Aug 30, 2016

Add support secondary arguments support to load_ui #81

Closed

mottosso reviewed Aug 30, 2016
View reviewed changes

fredrikaverpil added 2 commits August 31, 2016 09:18

Fix comments by @mottosso

659b32c

Also copy non-python example files into work directory

fb41189

fredrikaverpil added 2 commits August 31, 2016 09:46

Do not buld examples, just run them as they are

2d06076

Rename examples

f7d74c4

mottosso reviewed Aug 31, 2016
View reviewed changes

Wait for Xvfb

92bbf74

mottosso reviewed Aug 31, 2016
View reviewed changes

fredrikaverpil added 2 commits August 31, 2016 11:58

Convert tabs to spaces (oops)

d4b6e62

Add linebreaks for long bash command

6649b6b

fredrikaverpil merged commit 36effce into mottosso:master Aug 31, 2016

fredrikaverpil deleted the feature/examples branch August 31, 2016 10:53

fredrikaverpil mentioned this pull request Aug 31, 2016

Remove build_examples.py #126

Merged

Add examples #119

Add examples #119

Conversation

fredrikaverpil commented Aug 29, 2016

fredrikaverpil commented Aug 29, 2016 • edited Loading

mottosso commented Aug 29, 2016

fredrikaverpil commented Aug 29, 2016

fredrikaverpil commented Aug 29, 2016 • edited Loading

mottosso commented Aug 29, 2016

fredrikaverpil commented Aug 29, 2016

fredrikaverpil commented Aug 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikaverpil commented Aug 30, 2016

Choose a reason for hiding this comment

mottosso commented Aug 30, 2016

fredrikaverpil commented Aug 30, 2016

fredrikaverpil commented Aug 31, 2016 • edited Loading

fredrikaverpil commented Aug 31, 2016 • edited Loading

fredrikaverpil commented Aug 31, 2016

fredrikaverpil commented Aug 31, 2016 • edited Loading

mottosso commented Aug 31, 2016

fredrikaverpil commented Aug 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikaverpil Aug 31, 2016 • edited Loading

Choose a reason for hiding this comment

fredrikaverpil commented Aug 31, 2016 • edited Loading

Choose a reason for hiding this comment

fredrikaverpil Aug 31, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mottosso commented Aug 31, 2016

fredrikaverpil commented Aug 31, 2016

fredrikaverpil commented Aug 29, 2016 •

edited

Loading

fredrikaverpil commented Aug 29, 2016 •

edited

Loading

fredrikaverpil commented Aug 31, 2016 •

edited

Loading

fredrikaverpil commented Aug 31, 2016 •

edited

Loading

fredrikaverpil commented Aug 31, 2016 •

edited

Loading

fredrikaverpil Aug 31, 2016 •

edited

Loading

fredrikaverpil commented Aug 31, 2016 •

edited

Loading

fredrikaverpil Aug 31, 2016 •

edited

Loading