Project Refactoration #23

breno-jesus-fernandes · 2022-06-06T01:45:58Z

So How to improve the project?💡

My goal here it was to bring poetry to the battle ⚔️ . I need help to manage dependencies, test and deploy the project the easier way possible, the project is simple and should keep that way. So I bring everything inside pyproject.toml, so we do not need to worry anymore to be lost in thousands of config files and the develop dependencies are explicit declared separately. In resume, It becomes really easy to setup, tests and publish. I just adapted everything to poetry, include github actions.

poetry install -> make magic code 🪄 -> poetry run all_tests .... voilàà ( do you want to publish? poetry publish 😉)

Another important step it was to improve the project structure. Since PEP 561 the default structure to a stubs-only package is just "my_package-stubs", so i though better to bring the package to pandas-stubs and the tests to outside the pandas folder. Furthermore, I added a docs folder to documentation and a scripts folder to util scripts.

There is still a lot a work to make, but I hope with this strong base we could improve the project.

…kg structure now follows pep 561

twoertwein · 2022-06-06T16:25:19Z

.gitignore

@@ -127,3 +127,4 @@ dmypy.json

 # Pyre type checker
 .pyre/
+/poetry.lock


Probably not important for now: It could be helpful to add poetry.lock to the repository (poetry install will take less time and it improves reproducibility) but every now and then someone needs to update poetry.lock (is there a github bot for this?).

Hmm i see, probably a good ideia let poetry.lock available. But what do you mean with github bot, I didn't get the ideia.

If poetry.lock is added to the repository, it will eventually be out-of-date (for example not using the latest numpy). Instead of having to create a PR that updates poetry.lock, it would be handy if this process could be automated.

Oh indeed , looks like a headache hahha. I think is better to keep that way, It could cause problems in CI/CD, too.

twoertwein · 2022-06-06T16:29:37Z

pyproject.toml

+typing-extensions = ">=4.2.0"
+matplotlib = ">=3.3.2"
+pytest = ">=7.1.2"
+pyright = ">=1.1.251"


This "pyright" is not affiliated with Microsoft: https://pypi.org/project/pyright/

Does this "pyright" also install the actual pyright (which requires node)?

It's not affiliate but it is the same code, he just wraps it with python and runs node in the background. I think there is no problem.

twoertwein · 2022-06-07T21:08:00Z

I will approve this PR so that the CI can run (and then undo my approval - hope it doesn't cancel the CI run)

Dr-Irv · 2022-06-07T21:34:01Z

I will approve this PR so that the CI can run (and then undo my approval - hope it doesn't cancel the CI run)

I don't think you have to do this yet, because the workflow has been run in his github account. See https://github.com/BrenoJesusFernandes/pandas-stubs/actions

Dr-Irv

Thanks for this great contribution. We need to straighten out testing of the built wheel. See below. I will need to try out this PR soon so I can understand better how it builds a wheel we can give to pypi.org

Dr-Irv · 2022-06-07T21:38:44Z

.github/workflows/test.yml

-        run:
-          mypy tests/pandas
+    - name: Run MyPy
+      run: poetry run mypy pandas-stubs tests


One issue that I have run into with respect to using the more traditional method of a build using setup.py is that the wheel that is built, when installed, isn't passing the tests. So if you look at https://github.com/pandas-dev/pandas-stubs/blob/main/.github/workflows/test.yml , you will see this in there:

- name: Build wheel and install and remove typings run: | python setup.py build bdist_wheel find ./dist/*.whl | xargs pip install rm -rf typings pip install pyright # - name: run pyright command line # run: # pyright -p pyrightdistconfig.json --dependencies # - name: Run pyright against dist # uses: gramster/pyright-action@main # with: # project: pyrightdistconfig.json # extra-args: --dependencies # warn-partial: true - name: Run mypy against dist run: mypy tests/pandas

The problem right now is that pyright is failing with this, and there is a long discussion at microsoft/pyright#3540 , there seems to be an issue with the presence of the py.typed file that is interpreted differently on MacOS versus Windows and Linux.

In any case, adding the tests here to build the wheel, install it, and then test pyright and mypy against the installed wheel is something that should be done.

I had a lot of trouble figuring out the "py.typed" with "partial\n" string it was causing the problem, at least in my environment. not just in wheel installed case. So for a while, I just delete the file to show the poetry idea. I have the three Local OS here, I'm gonna test and see if I can help to fix the bug in the official pyright repository. To add the test with the wheel is not gonna be hard when the pyright is gonna work properly.

So it turns out that if I remove py.typed, then everything works locally and with the installed wheel. That version is now in main. So remove py.typed from your version and it should be OK

Dr-Irv · 2022-06-07T21:39:32Z

README.md

+    - 1 - How to setup the enviroment
+    - 2 - How to test the project
+    - 3 - How to follow the code style 
+    - 4 - Security stuffs
+    - 5 - How to publish


Can you create links from here to the docs you created?

Ohh yeah, it is easier to see.

Dr-Irv · 2022-06-07T21:47:11Z

pyproject.toml

@@ -1,3 +1,32 @@
+[tool.poetry]
+name = "pandas-stubs"
+version = "0.0.1"


I'd like to use a version numbering convention of pandas_version.yymmdd for the stubs. So if these were done today, the version would be 1.4.2.220607 . Then we mark the releases of the stubs by their release date, and also know which version of pandas the stubs correspond to.

I did not work with the poetry version yet, because we did not have a convention yet and my future ideia is to automate the version, tag, and publish system, so we do not need to worry with the static version in pyproject.toml file.

By the way, I really like your convention proposal, it is really easy. Maybe we could follow this approach.

Oh another important point to work is to upgrade the current stub version to work in 1.4.2 version, I guess you already saw it, but it is compatible just with API 1.2.x.

Oh another important point to work is to upgrade the current stub version to work in 1.4.2 version, I guess you already saw it, but it is compatible just with API 1.2.x.

What makes you believe it is not compatible with 1.4? Are we missing something?

The stubs that are here came from the Microsoft repo https://github.com/microsoft/python-type-stubs (and they just retired the pandas stubs there). I've used them successfully in projects using pandas 1.4.2, so I'd like to understand your comment about compatibility with API versions.

Dr-Irv · 2022-06-07T21:48:31Z

pyproject.toml

+]
+
+[tool.poetry.dependencies]
+python = ">=3.8,<4.0"


maybe we should say <=3.10 since we only support pandas up to that version

Nice, I did not even notice it. I'm gonna fix it.

Dr-Irv · 2022-06-08T14:51:56Z

One thing that is making this PR hard to evaluate is that you applied black and isort to the PYI files, which means many of them changed, and I think you may have added some PYI files that may not be necessary.

Can you split the PR into two PRs? One is just the poetry stuff and then another PR to do the black and isort part. Could be done in either order.

breno-jesus-fernandes · 2022-06-08T15:34:15Z

Sure no problem

* Add poetry, all configs and dependences now are in pyproject.toml, pckg structure now follows pep 561 * Fix: Dependencies versions * Feature: Scratch files to document the project * Feature: Improve CI/CD with poetry and removing pyright workaround * Fix: Add black and isort in dependencies * Feature: Add a script to run all tests, just type "poetry run all_test" * Feature: Add something in documentations * Fix: Project version, Author, Python versions, Pandas version * Fix: Remove old test file documentation * Fix: Remove old pyright configs * Fix: Documentation in README.md with links * Fix: pyproject.toml python compatibility * Fix: Add tests with wheel * Fix: Remove pytests from tests against dist * Fix: Remove Source code distribution on tests after install wheel * Fix: Using shutil to remove dir * Fix: Improve local tests and fix CI tests * Fix: Add metadata to pyproject.toml when build a wheel * Fix: Replace default "poetry.scripts" equivalent to console_scripts from setuptools to "poe" a poetry plugin to run util scripts. * Fix: Improving docs with instructions to update dependencies "poetry update". * Fix: Split Dev from Dist Dependencies * Refactor: Util test scripts * CI: All tests call poe to run * Docs: Improving setup and test documentation * Refactor: Improving test_src (I still think it could be better) * Docs: Improving poe tests documentation * Style: Apply black and isort to new scripts code * Fix: Bug when test_src code fails, test_dist continues to test * Fix: Error in test documentation * Docs: Improving CLI help documentation to run tests * Docs: Improving CLI help documentation to run tests * CI: Runs CI only when change code or dependencies * Docs: Fix Mc Donald's english errors * Fix: Package test * Fix: Package Test * CI: Fix package test * Fix: add project to sys path * Refactor: Folder Hierarquie, Rollback job, refresh setup docs * Fix: Add rollback to test_dist * Feature: Add code style check * Refactor: Improving rollback * Feature: add clean cache option in local tests * Fix: code style check Co-authored-by: Breno Fernandes <brenojesusfernandes@pm.com>

Breno Fernandes and others added 12 commits June 5, 2022 15:40

Add poetry, all configs and dependences now are in pyproject.toml, pc…

c9c3efa

…kg structure now follows pep 561

Fix: Dependencies versions

cc394fb

Feature: Scratch files to document the project

3dabd25

Feature: Improve CI/CD with poetry and removing pyright workaround

697ade8

Fix: Add black and isort in dependencies

1a2c94f

Feature: Add a script to run all tests, just type "poetry run all_test"

7176fd0

Feature: Add something in documentations

13b1b19

Fix: Apply black and isort style

b54faf5

Fix: Remove old tests doc

61b6537

Fix: Update README.md where is the documentation

aeda200

Fix: Update README.md where is the documentation

d82ab5b

Fix: Remove unecessary configs in pyright

bd98f44

breno-jesus-fernandes mentioned this pull request Jun 6, 2022

Proposal: One file to all dependencies and configs of the project #19

Closed

twoertwein reviewed Jun 6, 2022

View reviewed changes

Dr-Irv requested changes Jun 7, 2022

View reviewed changes

Dr-Irv mentioned this pull request Jun 8, 2022

Figure out how to package the stubs as a pandas-stubs project for distribution #4

Closed

breno-jesus-fernandes mentioned this pull request Jun 8, 2022

Add poetry and fix problem from #23 #25

Merged

breno-jesus-fernandes closed this Jun 8, 2022

breno-jesus-fernandes deleted the feature/project-refactoration branch June 10, 2022 22:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Project Refactoration #23

Project Refactoration #23

breno-jesus-fernandes commented Jun 6, 2022

twoertwein Jun 6, 2022

breno-jesus-fernandes Jun 6, 2022

twoertwein Jun 6, 2022

breno-jesus-fernandes Jun 6, 2022

twoertwein Jun 6, 2022

breno-jesus-fernandes Jun 6, 2022

twoertwein commented Jun 7, 2022

Dr-Irv commented Jun 7, 2022

Dr-Irv left a comment

Dr-Irv Jun 7, 2022

breno-jesus-fernandes Jun 8, 2022

Dr-Irv Jun 8, 2022

Dr-Irv Jun 7, 2022

breno-jesus-fernandes Jun 8, 2022

Dr-Irv Jun 7, 2022

breno-jesus-fernandes Jun 8, 2022

breno-jesus-fernandes Jun 8, 2022

breno-jesus-fernandes Jun 8, 2022

Dr-Irv Jun 8, 2022

Dr-Irv Jun 7, 2022

breno-jesus-fernandes Jun 8, 2022 •

edited

Loading

Dr-Irv commented Jun 8, 2022

breno-jesus-fernandes commented Jun 8, 2022

Project Refactoration #23

Project Refactoration #23

Conversation

breno-jesus-fernandes commented Jun 6, 2022

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

twoertwein commented Jun 7, 2022

Dr-Irv commented Jun 7, 2022

Dr-Irv left a 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

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

breno-jesus-fernandes Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Dr-Irv commented Jun 8, 2022

breno-jesus-fernandes commented Jun 8, 2022

breno-jesus-fernandes Jun 8, 2022 •

edited

Loading