Skip to content

Rename poe tasks #137

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

Merged
merged 15 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 6 additions & 18 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,29 +39,17 @@ jobs:
- name: Install project dependencies
run: poetry install -vvv --no-root

- name: Run MyPy Against Source Code
- name: Run mypy on 'tests' (using the local stubs) and on the local stubs
run: poetry run poe mypy_src

- name: Run Pyright Against Source Code
- name: Run pyright on 'tests' (using the local stubs) and on the local stubs
run: poetry run poe pyright_src

- name: Run Pytest Against Source Code
run: poetry run poe pytest_src

- name: Build Distribution
run: poetry run poe build_dist

- name: Install Distribution
run: poetry run poe install_dist
- name: Run pytest
run: poetry run poe pytest

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

- name: Rename Source Code
run: poetry run poe rename_src

- name: Run Pyright Against Distribution
run: poetry run poe pyright_dist

- name: Run MyPy Against Distribution
run: poetry run poe mypy_dist
- name: Install pandas-stubs and run tests on the installed stubs
run: poetry run poe test_dist
76 changes: 32 additions & 44 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,76 +50,64 @@ isort = ">=5.10.1"
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

[tool.poe.tasks.test_src]
help = "LOCAL Test | Run tests against source code version: \"poe test_src -p=<profile>\""
script = "scripts.test:test_src(profile, clean_cache)"
[tool.poe.tasks.test_all]
help = "Run all tests"
script = "scripts.test:test(clean_cache, src=True, dist=True)"

[[tool.poe.tasks.test_src.args]]
help = "\"default\" (mypy + pyright) or \"pytest\" (pytest only) or \"full\" (mypy + pyright + pytest)"
name = "profile"
options = ["-p", "--profile"]
default = "default"
[[tool.poe.tasks.test_all.args]]
help = "remove cache folders (mypy and pytest)"
name = "clean-cache"
options = ["-c", "--clean_cache"]
default = false

[tool.poe.tasks.test_src]
help = "Run local tests (includes 'mypy_src', 'pyright_src', 'pytest', and 'style')"
script = "scripts.test:test(clean_cache, src=True)"

[[tool.poe.tasks.test_src.args]]
help = "remove mypy and pytest cache folders"
help = "remove cache folders (mypy and pytest)"
name = "clean-cache"
options = ["-c", "--clean_cache"]
default = false

[tool.poe.tasks.test_dist]
help = "Local Test | Run tests against distribuition version: \"poe test_dist\""
script = "scripts.test:test_dist(clean_cache)"
help = "Run tests on the installed stubs (includes 'mypy_dist' and 'pyright_dist')"
script = "scripts.test:test(clean_cache, dist=True)"

[[tool.poe.tasks.test_dist.args]]
help = "remove mypy and pytest cache folders"
help = "remove cache folders (mypy and pytest)"
name = "clean-cache"
options = ["-c", "--clean_cache"]
default = false

[tool.poe.tasks.test_all]
help = "Local Test | Run tests against source code and distribuition version: \"poe test_all\""
script = "scripts.test:test_all(clean_cache)"
[tool.poe.tasks.pytest]
help = "Run pytest"
script = "scripts.test.run:pytest"

[[tool.poe.tasks.test_all.args]]
help = "remove mypy and pytest cache folders"
name = "clean-cache"
options = ["-c", "--clean_cache"]
default = false
[tool.poe.tasks.style]
help = "Run pre-commit"
script = "scripts.test.run:style"

[tool.poe.tasks.mypy_src]
help = "CI Test | Run mypy against source code"
help = "Run mypy on 'tests' (using the local stubs) and on the local stubs"
script = "scripts.test.run:mypy_src"

[tool.poe.tasks.mypy_dist]
help = "Run mypy on 'tests' using the installed stubs"
script = "scripts.test:test(clean_cache=False, dist=True, type_checker='mypy')"

[tool.poe.tasks.pyright_src]
help = "CI Test | Run pyright against source code"
help = "Run pyright on 'tests' (using the local stubs) and on the local stubs"
script = "scripts.test.run:pyright_src"

[tool.poe.tasks.pytest_src]
help = "CI Test | Run pytest against source code"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we keep this one in there? It's useful sometimes to just run pytest and not mypy and pyright ?

Copy link
Member Author

Choose a reason for hiding this comment

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

test_src has some redundancy with the other tests. Pytest can still be run with poe test_src --profile=pytest.

I think there are two options:

  1. add more options to test_src (for example only mypy and only pyright) and remove pytest_src, mypy_src, and pyright_src or
  2. keep pytest_src but use test_src only for running all local tests (remove mypy+pyright, only pytest, only pre-commit).

I slightly prefer option 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added pytest_src back and removed the profiles from test_src

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about this:

  • have test_src have no profile, do mypy, pyright, pytest and style
  • add back pytest_src
  • add style

then we have the following options:

  • local tests
    • test_src (does all of the next 4)
    • mypy_src
    • pyright_src
    • pytest_src
    • style
  • dist tests (each of the following builds the dist, installs the dist, renames the source, does the test(s), renames the source back, uninstalls the dist)
    • test_dist (does all of the next 3)
    • mypy_dist
    • pyright_dist
    • pytest_dist (should add that)

Don't think we need install_dist and rename_src as top level options.

Copy link
Member Author

Choose a reason for hiding this comment

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

* add `style`

Yeah, that is missing right now.

* dist tests (each of the following builds the dist, installs the dist, renames the source, does the test(s), renames the source back, uninstalls the dist)
  
  * `test_dist` (does all of the next 3)
  * `mypy_dist`
  * `pyright_dist`

I like that as it makes testing the installation easier while developing but it will make the CI slower (unless the CI just calls test_dist)

  * `pytest_dist` (should add that)

tests/ is not part of the wheel (and I think it is good that it isn't part of the wheel)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I like that as it makes testing the installation easier while developing but it will make the CI slower (unless the CI just calls test_dist)

I'm fine if the CI just calls test_dist or just calls the 3 individual pieces (which I think it does now)

tests/ is not part of the wheel (and I think it is good that it isn't part of the wheel)

If you look at the poe run, both mypy_dist and pyright_dist test the tests folder against the installed distribution, so we're fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now it looks like this:

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

looks good

script = "scripts.test.run:pytest_src"

[tool.poe.tasks.build_dist]
help = "CI Test | Build distribuition"
script = "scripts.test.run:build_dist"

[tool.poe.tasks.install_dist]
help = "CI Test | Install distribuition"
script = "scripts.test.run:install_dist"
[tool.poe.tasks.pyright_dist]
help = "Run pyright on 'tests' using the installed stubs"
script = "scripts.test:test(clean_cache=False, dist=True, type_checker='pyright')"

[tool.poe.tasks.rename_src]
help = "CI Test | Rename source code"
script = "scripts.test.run:rename_src"

[tool.poe.tasks.mypy_dist]
help = "CI Test | Run mypy against distribuition"
script = "scripts.test.run:mypy_dist"

[tool.poe.tasks.pyright_dist]
help = "CI Test | Run pyright against distribuition"
script = "scripts.test.run:pyright_dist"

[tool.black]
target-version = ['py38', 'py39']
target-version = ['py38']

[tool.isort]
known_pre_libs = "pandas._config"
Expand Down
102 changes: 30 additions & 72 deletions scripts/test/__init__.py
Original file line number Diff line number Diff line change
@@ -1,82 +1,40 @@
from typing import Literal

from scripts._job import run_job
from scripts.test import _step


def test_src(profile: str, clean_cache: bool = False):
steps = []
if clean_cache:
steps.extend(
[
_step.clean_mypy_cache,
_step.clean_pytest_cache,
]
)

if profile in (None, "", "default"):
steps.extend([_step.mypy_src, _step.pyright_src])
elif profile == "pytest":
steps.extend([_step.pytest_src])
elif profile == "style":
steps.extend([_step.style_src])
elif profile == "full":
steps.extend(
[_step.mypy_src, _step.pyright_src, _step.pytest_src, _step.style_src]
)
else:
raise ModuleNotFoundError("Profile not found!")

run_job(steps)


def test_dist(clean_cache: bool = False):
_CACHE_STEPS = [_step.clean_mypy_cache, _step.clean_pytest_cache]
_SRC_STEPS = [_step.mypy_src, _step.pyright_src, _step.pytest, _step.style]
_DIST_STEPS = [
_step.build_dist,
_step.install_dist,
_step.rename_src,
_step.mypy_dist,
_step.pyright_dist,
_step.uninstall_dist,
_step.restore_src,
]


def test(
clean_cache: bool = False,
src: bool = False,
dist: bool = False,
type_checker: Literal["", "mypy", "pyright"] = "",
):
steps = []
if clean_cache:
steps.extend(
[
_step.clean_mypy_cache,
_step.clean_pytest_cache,
]
)
steps.extend(_CACHE_STEPS)

steps.extend(
[
_step.build_dist,
_step.install_dist,
_step.rename_src,
_step.mypy_dist,
_step.pyright_dist,
_step.uninstall_dist,
_step.restore_src,
]
)

run_job(steps)
if src:
steps.extend(_SRC_STEPS)

if dist:
steps.extend(_DIST_STEPS)

def test_all(clean_cache: bool = False):
steps = []
if clean_cache:
steps.extend(
[
_step.clean_mypy_cache,
_step.clean_pytest_cache,
]
)

steps.extend(
[
_step.mypy_src,
_step.pyright_src,
_step.pytest_src,
_step.style_src,
_step.build_dist,
_step.install_dist,
_step.rename_src,
_step.mypy_dist,
_step.pyright_dist,
_step.uninstall_dist,
_step.restore_src,
]
)
if type_checker:
# either pyright or mypy
remove = "mypy" if type_checker == "pyright" else "pyright"
steps = [step for step in steps if remove not in step.name]

run_job(steps)
32 changes: 21 additions & 11 deletions scripts/test/_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,30 @@

clean_mypy_cache = Step(name="Clean mypy cache", run=run.clean_mypy_cache)
clean_pytest_cache = Step(name="Clean pytest cache", run=run.clean_pytest_cache)
mypy_src = Step(name="Run Mypy Against Source Code", run=run.mypy_src)
pyright_src = Step(name="Run Pyright Against Source Code", run=run.pyright_src)
pytest_src = Step(name="Run Pytest Against Source Code", run=run.pytest_src)
style_src = Step(name="Run pre-commit Against Source Code", run=run.style_src)
build_dist = Step(name="Build Dist", run=run.build_dist)
mypy_src = Step(
name="Run mypy on 'tests' (using the local stubs) and on the local stubs",
run=run.mypy_src,
)
pyright_src = Step(
name="Run pyright on 'tests' (using the local stubs) and on the local stubs",
run=run.pyright_src,
)
pytest = Step(name="Run pytest", run=run.pytest)
style = Step(name="Run pre-commit", run=run.style)
build_dist = Step(name="Build pandas-stubs", run=run.build_dist)
install_dist = Step(
name="Install Dist", run=run.install_dist, rollback=run.uninstall_dist
name="Install pandas-stubs", run=run.install_dist, rollback=run.uninstall_dist
)
rename_src = Step(
name="Rename Source Code Folder",
name="Rename local stubs",
run=run.rename_src,
rollback=run.restore_src,
)
mypy_dist = Step(name="Run MyPy Against Dist", run=run.mypy_dist)
pyright_dist = Step(name="Run Pyright Against Dist", run=run.pyright_dist)
uninstall_dist = Step(name="Uninstall Dist", run=run.uninstall_dist)
restore_src = Step(name="Restore Source Code", run=run.restore_src)
mypy_dist = Step(
name="Run mypy on 'tests' using the installed stubs", run=run.mypy_dist
)
pyright_dist = Step(
name="Run pyright on 'tests' using the installed stubs", run=run.pyright_dist
)
uninstall_dist = Step(name="Uninstall pandas-stubs", run=run.uninstall_dist)
restore_src = Step(name="Restore local stubs", run=run.restore_src)
8 changes: 4 additions & 4 deletions scripts/test/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ def pyright_src():
subprocess.run(cmd, check=True)


def pytest_src():
def pytest():
cmd = ["pytest"]
subprocess.run(cmd, check=True)


def style_src():
def style():
cmd = ["pre-commit", "run", "--all-files", "--verbose"]
subprocess.run(cmd, check=True)

Expand All @@ -29,8 +29,8 @@ def build_dist():


def install_dist():
path = next(Path("dist/").glob("*.whl"))
cmd = ["pip", "install", str(path)]
path = sorted(Path("dist/").glob("pandas_stubs-*.whl"))[-1]
cmd = ["pip", "install", "--force-reinstall", str(path)]
subprocess.run(cmd, check=True)


Expand Down