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.

Sign up for GitHub

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

Jump to bottom

Improve documentation of CONTRIBUTING.md #6

Merged
merged 5 commits into from
Jul 30, 2023
Merged

Improve documentation of CONTRIBUTING.md #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4b3f98
fixing documentation of `CONTRIBUTING.md`
Diogo-Rossi Jul 27, 2023
6d3fd5b
add note to install requirements
Diogo-Rossi Jul 27, 2023
249c486
small fixing of docs
Diogo-Rossi Jul 27, 2023
132408b
correct typo
Diogo-Rossi Jul 28, 2023
af68b35
add `**/__pycache__` to `.gitignore`
Diogo-Rossi Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
/dist/
/PySide2_stubs.egg-info
.mypy_cache
**/__pycache__
59 changes: 33 additions & 26 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,39 @@ The CI way

This is the easiest/quickest way in terms of setup.

1. Clone pyside2-stubs
1. Create a fork of `pyside2-stubs`


2. Add test showing the incorrect stub. For example, I noticed that QAction.setShortcut() was declared to support
only QKeySequence whereas a simple string is also supported.
2. Clone your forked repo of `pyside2-stubs`

So I created a new file tests\qaction.py using the correct code usage, but detected incorrectly by mypy:

3. Add test showing the incorrect stub. For example, I noticed that `QAction.setShortcut()` was declared to support
only `QKeySequence` whereas a simple string is also supported.

So I created a new file `tests\qaction.py` using the correct code usage, but detected incorrectly by mypy:

from PySide2.QtWidgets import QAction

a = QAction()
a.setShortcut('Ctrl+F')


3. Fix the PySide2 stubs. In my case, I changed the signature of QAction.setShortcut()
4. Fix the PySide2 stubs. In my case, I changed the signature of `QAction.setShortcut()`

from:


def setShortcut(a: QKeySequence) -> None: ...
def setShortcut(a: QKeySequence) -> None: ...

to:

def setShortcut(a: typing.Union[QKeySequence, str]) -> None: ...
def setShortcut(a: typing.Union[QKeySequence, str]) -> None: ...


4. Commit the change and push it to your repo.
5. Commit the change and push it to your repo.


5. Create a pull request against pyside2-stubs. The PR will run the CI for your change and ensure that all tests continue
6. Create a pull request against `pyside2-stubs`. The PR will run the CI for your change and ensure that all tests continue
to pass successfully. I'll be happy to merge the result.


Expand All @@ -51,32 +54,36 @@ This is the easiest/quickest way in terms of setup.
The manual way
--------------

Here, you take all the steps to reproduce a complete stub testing environement to include your change.
Here, you take all the steps to reproduce a complete stub testing environment to include your change.

1. Create a fork of `pyside2-stubs`

1. Clone pyside2-stubs
2. Clone your forked repo of `pyside2-stubs`

2. Create a virtual environment (I used .env as a name for my virtual environment) and activate it.
3. Create a virtual environment (I used `.env` as a name for my virtual environment) and activate it.


$ python -m venv .env
$ .env\scripts\activate
python -m venv .env
.env\scripts\activate


3. Install the required packages for running the tests.
4. Install the required packages for running the tests.

pip install -r dev-requirements.txt
python -m pip install -r dev-requirements.txt

Note: running pip as a python module is necessary because `dev-requirements.txt` modify pip itself.
This can be confirmed by trying to run `pip install -r dev-requirements.txt`

4. Make your current directory an editable installed package.
5. Make your current directory an editable installed package.


pip install -e .
pip install -e .

Note: currently, with pip 22 and above, and mypy 0.971, the editable installation is not picked by mypy.
For the CI, I need to force pip to version 21.2 for it to work. You may need to force pip also to this version.


5. Make sure the tests run correctly as they are:
6. Make sure the tests run correctly as they are:

$ pytest -v
[...]
Expand All @@ -86,7 +93,7 @@ Here, you take all the steps to reproduce a complete stub testing environement t
There should be no failure reported.


6. Add test showing the incorrect stub definitions. For example, I noticed that `QAction.setShortcut()` was declared to support
7. Add test showing the incorrect stub definitions. For example, I noticed that `QAction.setShortcut()` was declared to support
only `QKeySequence` whereas a simple string is also supported.

So I created a new file `tests\qaction.py` using the correct code usage, but detected incorrectly by mypy:
Expand All @@ -97,7 +104,7 @@ Here, you take all the steps to reproduce a complete stub testing environement t
a.setShortcut('Ctrl+F')


7. Run the tests again. Your new file should be picked up and reported as an error by the mypy test.
8. Run the tests again. Your new file should be picked up and reported as an error by the mypy test.


(.env2) c:\oss\PyQt-stubs\PySide2-stubs>pytest -v
Expand Down Expand Up @@ -148,10 +155,10 @@ Here, you take all the steps to reproduce a complete stub testing environement t
(.env2) c:\oss\PyQt-stubs\PySide2-stubs>


Note that you can run your new test only by uing: pytest -k "your test name" -v
Note that you can run your new test only by using: `pytest -k "your test name" -v`


8. Fix the PySide2 stubs. In my case, I changed the signature of QAction.setShortcut()
9. Fix the PySide2 stubs. In my case, I changed the signature of `QAction.setShortcut()`

from:

Expand All @@ -162,14 +169,14 @@ Here, you take all the steps to reproduce a complete stub testing environement t
def setShortcut(a: typing.Union[QKeySequence, str]) -> None: ...


9. Rerun the tests. If it looks like mypy is still using the old stubs, delete .mypy-cache . It helps.
10. Rerun the tests. If it looks like mypy is still using the old stubs, delete `.mypy-cache` . It helps.
You should now see no error, but two more tests successful.

$ pytest -v
[...]
================================================= 62 passed in 13.07s =================================================
$

10. Commit your changes and push them to your repository.
11. Commit your changes and push them to your repository.

11. Create a pull request. I'll be glad to merge it. The CI will run basically the same tests that you did over your changes.
12. Create a pull request. I'll be glad to merge it. The CI will run basically the same tests that you did over your changes.