Skip to content

Fix CI for Windows #242

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

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
b24f21a
test on Windows with 3.10, unpin poetry
Dr-Irv Aug 31, 2022
d6bfb37
remove caching of poetry.lock
Dr-Irv Aug 31, 2022
8e7782a
try simple testing program
Dr-Irv Aug 31, 2022
2902589
try pandas in testing
Dr-Irv Aug 31, 2022
cc50504
turn off complex tests. Remove 50% of dependencies
Dr-Irv Aug 31, 2022
e7a0d84
remove all dependencies except pandas
Dr-Irv Aug 31, 2022
f840ad2
avoid poetry
Dr-Irv Aug 31, 2022
f01da8c
poetry install remove no-root
Dr-Irv Aug 31, 2022
860b2fa
where is python
Dr-Irv Aug 31, 2022
2fcff6c
run tester using poetry
Dr-Irv Aug 31, 2022
1d0270a
put back poethepoet
Dr-Irv Aug 31, 2022
eb74597
remove loguru dependence
Dr-Irv Aug 31, 2022
df77d18
try using setup-python@main action
Dr-Irv Aug 31, 2022
67c078a
poetry env use system
Dr-Irv Aug 31, 2022
fe0b779
create a virtual environment
Dr-Irv Aug 31, 2022
7967bd2
get activation right
Dr-Irv Aug 31, 2022
aa5b73b
move venv after poetry install and print info
Dr-Irv Aug 31, 2022
f3f1f49
use poetry env use system throughout
Dr-Irv Sep 1, 2022
bef2ccc
try windows 2019
Dr-Irv Sep 1, 2022
097b85f
try using github action for poetry
Dr-Irv Sep 1, 2022
ca5e242
set shell to bash
Dr-Irv Sep 1, 2022
299b8d4
print sys.executable
Dr-Irv Sep 1, 2022
ade4669
use poe directly rather than poetry run
Dr-Irv Sep 1, 2022
0707b21
printenv, which python, which poe
Dr-Irv Sep 1, 2022
15a3b12
try python testing.py rather than poe
Dr-Irv Sep 1, 2022
cc9d3f9
put back mypy
Dr-Irv Sep 1, 2022
2a391f9
add matplotlib
Dr-Irv Sep 1, 2022
b7d9ff4
put back all dependencies and all tests
Dr-Irv Sep 1, 2022
c4f36a9
put back loguru. Try dist using python instead of sys.executable
Dr-Irv Sep 1, 2022
c900da0
set the poe executor to be the virtual env
Dr-Irv Sep 1, 2022
efaf430
use pip show to see where things went
Dr-Irv Sep 1, 2022
30cb06f
include the subprocess.run for pip show
Dr-Irv Sep 1, 2022
f2114e4
put all the dist tests in test.yaml
Dr-Irv Sep 1, 2022
6047947
refer to dist tests correctly
Dr-Irv Sep 1, 2022
9b302f6
avoid poetry for mypy. check if pandas_stubs is there
Dr-Irv Sep 1, 2022
b27cecb
show location of installation
Dr-Irv Sep 1, 2022
eceb862
build wheel and install
Dr-Irv Sep 1, 2022
48a008d
use unix path for dist, put run back to where it was
Dr-Irv Sep 1, 2022
a895e18
version in pyproject is 999999
Dr-Irv Sep 1, 2022
c580318
try the whole thing with cache
Dr-Irv Sep 1, 2022
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
47 changes: 33 additions & 14 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ jobs:
os: [ubuntu-latest, windows-latest, macos-latest]
python-version: ['3.8', '3.9', '3.10']

defaults:
run:
shell: bash

steps:

- uses: actions/checkout@v3
Expand All @@ -22,32 +26,47 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Install Poetry
run: pip install poetry==1.1.15

- name: Determine poetry version
run: echo "::set-output name=VERSION::$(poetry --version)"
id: poetry_version

- name: Cache poetry.lock
uses: actions/cache@v3
uses: snok/install-poetry@v1
with:
path: poetry.lock
key: ${{ matrix.os }}-${{ matrix.python-version }}-poetry-${{ steps.poetry_version.outputs.VERSION }}-${{ hashFiles('pyproject.toml') }}
virtualenvs-create: true
virtualenvs-in-project: true

- name: Load cached venv
id: cached-pip-wheels
uses: actions/cache@v2
with:
path: ~/.cache
key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ hashFiles('**/poetry.lock') }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this key correct, the CI says it is venv-Linux--?

This caches the (large) venv but I think it does not cache poetry.lock.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I picked this up from https://github.com/snok/install-poetry#running-on-windows

There are comments there that say that caching doesn't work on Windows.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the file poetry.lock exist on the CI? The previous caching used pyproject.toml as the key to cache poety.lock

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know. After spending hours on this last night, I was just happy to get it working.


- name: Install project dependencies
run: poetry install -vvv --no-root

- name: Run mypy on 'tests' (using the local stubs) and on the local stubs
run: poetry run poe mypy
run: |
source $VENV
poetry run poe mypy

- name: Run pyright on 'tests' (using the local stubs) and on the local stubs
run: poetry run poe pyright
run: |
source $VENV
poetry run poe pyright

- name: Run pytest
run: poetry run poe pytest
run: |
source $VENV
poetry run poe pytest

- if: matrix.python-version == '3.8' && matrix.os == 'ubuntu-latest'
uses: pre-commit/[email protected]

- name: Install pandas-stubs and run tests on the installed stubs
run: poetry run poe test_dist
run: |
source $VENV
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the above steps we can use poe ..., why can't we use it here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, if I use the run script, it installs the wheel into the "system" environment, not poetry's virtual environment.

poetry build -f wheel
python -m pip install --find-links=./dist --force-reinstall pandas-stubs
mv pandas-stubs _pandas-stubs
mypy tests --no-incremental
pyright tests
mv _pandas-stubs pandas-stubs


31 changes: 31 additions & 0 deletions pandas-stubs/core/dtypes/generic.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
def create_pandas_abc_type(name, attr, comp): ...

class ABCIndex: ...
class ABCInt64Index: ...
class ABCUInt64Index: ...
class ABCRangeIndex: ...
class ABCFloat64Index: ...
class ABCMultiIndex: ...
class ABCDatetimeIndex: ...
class ABCTimedeltaIndex: ...
class ABCPeriodIndex: ...
class ABCCategoricalIndex: ...
class ABCIntervalIndex: ...
class ABCIndexClass: ...
class ABCSeries: ...
class ABCDataFrame: ...
class ABCSparseArray: ...
class ABCCategorical: ...
class ABCDatetimeArray: ...
class ABCTimedeltaArray: ...
class ABCPeriodArray: ...
class ABCPeriod: ...
class ABCDateOffset: ...
class ABCInterval: ...
class ABCExtensionArray: ...
class ABCPandasArray: ...

class _ABCGeneric(type):
def __instancecheck__(cls, inst) -> bool: ...

class ABCGeneric: ...
3 changes: 3 additions & 0 deletions pandas-stubs/core/groupby/indexing.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class GroupByIndexingMixin: ...
class GroupByPositionalSelector: ...
class GroupByNthSelector: ...
6 changes: 5 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[tool.poetry]
name = "pandas-stubs"
version = "1.4.3.220829"
# Use the big version number so in CI, it will use the built wheel
version = "1.4.3.999999"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that needed? To invalidate the cache?

This should already pick the newest wheel:

path = sorted(Path("dist/").glob("pandas_stubs-*.whl"))[-1]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that if I build and install the wheel from the run script, it gets installed into the wrong python environment. So now with test.yaml doing a direct build of the wheel and installation, I have to force it to install the newly built wheel, which requires a higher version number than what is available on pypi.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which requires a higher version number than what is available on pypi.

So pip is preferring the pypi.org wheel over the local wheel? There must be an option to ignore the remote wheel.

Could also simply bump the version number after each release so that the CI works but as soon as we forget it pip will install the wrong pandas-stubs - seems a bit fragile.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue here is that we need this to work for PR's, which means that contributors have to change the version number.

In the test.yaml, I am now doing this:

python -m pip install --find-links=./dist --force-reinstall pandas-stubs

The --find-links=./dist makes it so that pip will look in the dist directory. On Windows, you can't do pip install dist/*, so this is the workaround for that.

description = "Type annotations for pandas"
authors = ["The Pandas Development Team <[email protected]>"]
license = "BSD-3-Clause"
Expand Down Expand Up @@ -56,6 +57,9 @@ pyreadstat = ">=1.1.9"
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

[tool.poe.executor]
type = "virtualenv"

[tool.poe.tasks.test_all]
help = "Run all tests"
script = "scripts.test:test(src=True, dist=True)"
Expand Down