-
-
Notifications
You must be signed in to change notification settings - Fork 136
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.
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
Project Refactoration #23
Changes from all commits
c9c3efa
cc394fb
3dabd25
697ade8
1a2c94f
7176fd0
13b1b19
b54faf5
61b6537
aeda200
d82ab5b
bd98f44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,3 +127,4 @@ dmypy.json | |
|
||
# Pyre type checker | ||
.pyre/ | ||
/poetry.lock | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not important for now: It could be helpful to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh indeed , looks like a headache hahha. I think is better to keep that way, It could cause problems in CI/CD, too. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,14 @@ This is the home for pandas typing stubs supported by the pandas core team. The | |
|
||
To contribute changes to the stubs, you must include an appropriate test. See `pandas-stubs/tests` for examples. | ||
|
||
## Build instructions | ||
### Documentation | ||
|
||
Use `python setup.py build bdist_wheel` to build the wheel. NOTE: `setuptools 62.3.2` is required! | ||
- Enter in docs to see: | ||
- 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 | ||
Comment on lines
+14
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you create links from here to the docs you created? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohh yeah, it is easier to see. |
||
|
||
## Evolution | ||
|
||
|
@@ -23,3 +28,4 @@ As both projects move forward, this page will track what the differences are (if | |
We are indebted to Microsoft and that project for the initial set of public type stubs. We are also grateful for the original pandas-stubs project at <https://github.com/VirtusLab/pandas-stubs> that created the framework for testing the stubs. | ||
|
||
Last update to README: 6/4/2022: 11:35 EDT | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
## Setup Environment | ||
|
||
- Make sure you have python >= 3.8 installed | ||
- Install poetry if you still don't have: pip install poetry | ||
- Install the project dependencies with: poetry install -vvv | ||
- Run all tests to make sure the project is ok: poetry run all_tests | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
## Test | ||
|
||
- These tests originally came from https://github.com/VirtusLab/pandas-stubs. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
## Code style | ||
|
||
- It's important to follow the code style from the project: | ||
- poetry run black pandas-stubs tests scripts | ||
- poetry run isort pandas-stubs tests scripts |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
## Security | ||
|
||
- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
## Publish | ||
|
||
You know ... just type "poetry publish pandas-stubs" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from __future__ import annotations | ||
|
||
from typing import Union | ||
|
||
class NAType: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from __future__ import annotations | ||
|
||
from datetime import datetime | ||
from typing import Any | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: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 thepy.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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it turns out that if I remove
py.typed
, then everything works locally and with the installed wheel. That version is now inmain
. So removepy.typed
from your version and it should be OK