Qt.py icon indicating copy to clipboard operation
Qt.py copied to clipboard

Add pyi stubs that redirect to PySide2

Open chadrik opened this issue 1 year ago • 2 comments

Since there are a number of competing stubs for PySide out there (my contribution is here), and someone may prefer to pip install one stub package over another, I propose creating a set of .pyi files that defer to PySide2, since that's the layout that Qt.py standardizes everything to.

After this change, installing Qt.py with type support would be done like this:

pip install Qt.py types-PySide2

chadrik avatar Aug 22 '22 16:08 chadrik

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 22 '22 16:08 CLAassistant

I think this is all ready to go. The build jobs are failing, but they seem unrelated. Any suggestions?

chadrik avatar Sep 19 '22 16:09 chadrik

Hey, sorry for the delay.

I had a look at the breaking tests, which led me to take a closer look at the changes in this PR, and it's turned Qt.py from a module to a package! That's not something we want to do; one of the USPs of Qt.py is that it is a self-contained single file. Once we head down the package route we'll introduce a series of other questions too, like why not separate QtCore, QtWidgets etc. into separate modules too?

Is there any way of achieving the goal of these stubs without having any effect on Qt.py the module?

mottosso avatar Sep 25 '22 13:09 mottosso

That's my bad, I should have known better than to make a change that drastic.

pep 561 doesn't directly discuss how to handle a module whose stubs are a package, so what I've done is to structure the stubs as a "stubs only" package, which is intended to be distributed as a separately installable package, but there's nothing stopping us distributing both module and stubs-only package from a single wheel. I built and tested this and it works.

Here's how I tested:

python3 setup.py bdist_wheel
mkdir stubstest
cd stubstest
python3 venv .venv
. .venv/bin/activate
pip install mypy types-PySide2 ../dist/Qt.py-1.3.7-py2.py3-none-any.whl
echo $'from Qt import QtCore, QtWidgets\nw = QtWidgets.QWidget()' > qtstubtest.py
mypy qtstubtest.py

For the record, I tried making an all-in-one Qt.pyi file, but I can't use a single module to properly define the Qt.Qt sub-module, which imports the contents of numerous other modules.

chadrik avatar Sep 26 '22 00:09 chadrik

I also added the feature to easily install PySide2 stubs via pip install Qt.py[stubs]

chadrik avatar Sep 26 '22 01:09 chadrik

One more thought here:

I think we should add types-PySide2 as a default requirement, and here's why:

  • I added something to the readme explaining how to get code completion working (pip install Qt.py[stubs]) but the reality is that 99% of people who experience broken code completion for Qt.py will not read this documentation, they'll simply be frustrated that it doesn't work out of the box.
  • types-PySide2 does not have any impact on runtime behavior: it's just a package of .pyi files. Qt.py will not generate any errors if the Qt-stubs or types-PySide2 packages are missing. These stub packages exist solely for code completion and static analysis.
  • Those people who are pip installing their packages have already accepted the added complexity of a virtual environment and adding types-PySide2 isn't going to add any additional complexity or issues.
  • Those who install Qt.py as a simple drop-in module continue to use exactly the same workflow. Adding this new requirement doesn't affect that workflow at all.

Ultimately it doesn't matter to me that much, but I thought I'd put this info out there so that you can make the call. Let me know what you'd like to do.

chadrik avatar Sep 26 '22 16:09 chadrik

Thanks, I can see what you mean and I agree.

Advanced users can still copy/paste the source of Qt.py into their own projects which is how I normally use it. And that those who don't care for or would use it still get auto-completion during a pip install seems fine; does no harm.

I was going to suggest we add tests for this; such that we know that when Travis is happy, the auto-completion mechanic is working as intended. But, thinking about it, I'm not sure how to test for it? 🤔 If we can't then fair enough. If you're happy with the functionality, and preferably one more third-party - such as @fredrikaverpil, @matiascodesal or @MaximeEngel - then I'm happy to merge this.

mottosso avatar Sep 26 '22 16:09 mottosso

I was going to suggest we add tests for this

I was hoping to do this as well. It looks like all of the test machinery is currently built around testing the Qt.py module "in situ" so it would require some reworking, because I think that the Qt-stubs package will only work if it's been installed into a site-packages directory. The best way to to test this is to build a wheel of Qt.py, pip install it into a virtualenv, and run mypy (we will need to restrict it to a version that's compatible with python 3.6).

Tox would be a good candidate for test runner, since by default tox pip installs your package into a tmp venv before running the test commands, which ensures that your setup.py is working and installs the other specified requirements. Tox is higher level than nose or pytest: it is designed to run multiple test commands with differing entry points, requirements, python versions, etc. So we would convert these tests into tox "environments":

printf "#\n# Running tests in Python ${PYTHON}\n"
export NOSETESTS_BINARY=nosetests${PYTHON}
printf "#\n# Testing implementation..\n"
    python${PYTHON} -u run_tests.py
printf "#\n# Testing caveats..\n"
    python${PYTHON} build_caveats.py
    nosetests${PYTHON} \
        --verbose \
        --with-doctest \
        --with-process-isolation \
        test_caveats.py
printf "#\n# Testing examples..\n"
    nosetests${PYTHON} \
    --verbose \
    --with-process-isolation \
    --with-doctest \
    --exe \
        examples/*/*.py

printf Done

Then we would add a new one with a mypy test.

I'm a little wary of wading into this, since it won't be a small project.

chadrik avatar Sep 26 '22 17:09 chadrik

Hi guys, 👋 long time 😄

I haven't used Qt.py in projects for several years now, so I'm not fully in the loop here. But with the history of Qt.py being a "standalone" module and easy to vendor I think your solution is great @chadrik. However, I would personally not use setup.py anymore now that pyproject.toml supports all of this out of the box - and removes all the remote code execution risks that comes with setup.py. But that's out of scope for this PR.

Adding working completion will bring a lot of value to end users and I think it's valuable to merge this in even if it does not contain tests. @mottosso hacktoberfest is coming up, maybe adding tests for this could be something to request from people seeking to contribute? But I'm not entirely sure how to test this and what to include in the scope. Maybe it would be sufficient to just verify that the intended functionality even works at all by verifying completion works for one function call. Like a minimal sanity check.

Regardless, Qt.py has always relied partly on user input on missing or wrong PySide2 attributes/methods. I think this will partly help keep these stubs updated.

Even if I'm really passive in this project these days, and for whatever it's worth, I'm all in favour for merging this in as I can't imagine coding "in the dark" like I used to do back in 2016 🥲 Nice work @chadrik!

fredrikaverpil avatar Sep 29 '22 06:09 fredrikaverpil

as I can't imagine coding "in the dark" like I used to do back in 2016

Ouch! 😅 Thanks for sharing your thoughts!

hacktoberfest is coming up, maybe adding tests for this could be something to request from people seeking to contribute?

Interesting, I'm not in the loop on that. Would you be able to organise?

mottosso avatar Sep 29 '22 08:09 mottosso

Just tag the repo with the hacktoberfest topic.

You can actively register the repo at https://hacktoberfest.com and formally register the repo as "eligible" or even register to host an event.

  • Register anytime between September 26 and October 31

  • Pull requests can be made in any GITHUB or GITLAB hosted project that’s participating in Hacktoberfest (look for the “hacktoberfest” topic)

  • Project maintainers must accept your pull/merge requests for them to count toward your total

  • Have 4 pull/merge requests accepted between October 1 and October 31 to complete Hacktoberfest

  • The first 40,000 participants (maintainers and contributors) who complete Hacktoberfest can elect to receive one of two prizes: a tree planted in their name, or the Hacktoberfest 2022 t-shirt.

-- https://hacktoberfest.com/participation/

I don't feel active around here to do any of this though (there might be follow-up questions), sorry.

fredrikaverpil avatar Sep 29 '22 12:09 fredrikaverpil

Ok, I pushed up the change to make types-PySide2 an install requirement. Let me know if you want me to squash my commits.

Since this is a big packaging change it might be good to push to test.pypi.org first and see if folks on the original thread want to try it out.

as I can't imagine coding "in the dark" like I used to do back in 2016

Ouch! 😅 Thanks for sharing your thoughts!

I think Fredrik is referring to coding without type stubs/annotations. Correct?

However, I would personally not use setup.py anymore now that pyproject.toml supports all of this out of the box - and removes all the remote code execution risks that comes with setup.py.

pyproject.toml has its benefits, but the code execution risks are already mitigated by the use of wheels.

But that's out of scope for this PR.

agreed.

chadrik avatar Sep 29 '22 18:09 chadrik

hacktoberfest is coming up, maybe adding tests for this could be something to request from people seeking to contribute?

I think it would be simpler if you framed the task as porting the tests to tox -- and addressing how that affects the docker containers and the entrypoint script. Once that's done I'd be happy to add a mypy test, as it'll be a 30 minute fix for someone who knows their way around tox and mypy.

chadrik avatar Sep 29 '22 18:09 chadrik

I think Fredrik is referring to coding without type stubs/annotations. Correct?

👍 😄

fredrikaverpil avatar Sep 30 '22 06:09 fredrikaverpil

I think Fredrik is referring to coding without type stubs/annotations. Correct?

Haha, yes. I'm the one in the dark ages still.

mottosso avatar Sep 30 '22 08:09 mottosso