From f7dcfb2e6ec55aad8e98466ee6b3ba7e81418b4a Mon Sep 17 00:00:00 2001 From: Alvaro Valdebenito Date: Mon, 20 Mar 2023 14:00:38 +0100 Subject: [PATCH 1/5] CO vs Co test case --- tests/test_cli.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 0639e5e..9fccac8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,4 +1,7 @@ +from __future__ import annotations + import pytest +from typer import Typer from typer.testing import CliRunner from airbase import __version__ @@ -23,3 +26,21 @@ def test_version(options: str): assert result.exit_code == 0 assert "airbase" in result.output assert __version__ in result.output + + +@pytest.mark.xfail( + reason="typer has problems with Enum cases https://github.com/tiangolo/typer/discussions/570", + strict=True, +) +def test_CO_vs_Co(): + app = Typer() + + pollutants = set() + + @app.command() + def CO_vs_Co(poll: Pollutant): + pollutants.add(poll) + + result = runner.invoke(app, ["CO"]) + assert result.exit_code == 0 + assert pollutants == {Pollutant.CO} From 7e4d4836556644fde15092fd51b666034e8d0482 Mon Sep 17 00:00:00 2001 From: Alvaro Valdebenito Date: Mon, 20 Mar 2023 14:41:11 +0100 Subject: [PATCH 2/5] skip CLI options that will confuse typer --- airbase/cli.py | 8 ++++---- tests/test_cli.py | 6 +----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/airbase/cli.py b/airbase/cli.py index ca61a9a..b775a22 100644 --- a/airbase/cli.py +++ b/airbase/cli.py @@ -10,10 +10,7 @@ from . import __version__ from .airbase import AirbaseClient -main = typer.Typer( - no_args_is_help=True, - add_completion=False, -) +main = typer.Typer(no_args_is_help=True, add_completion=False) client = AirbaseClient() @@ -33,6 +30,9 @@ class Pollutant(str, Enum): Pollutant = vars() for poll in client._pollutants_ids: + if poll.upper() in set(Pollutant): + # skipp options that will confuse typer + continue Pollutant[poll] = poll def __str__(self) -> str: diff --git a/tests/test_cli.py b/tests/test_cli.py index 9fccac8..e7ad2a5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,13 +25,9 @@ def test_version(options: str): result = runner.invoke(main, options.split()) assert result.exit_code == 0 assert "airbase" in result.output - assert __version__ in result.output + assert str(__version__) in result.output -@pytest.mark.xfail( - reason="typer has problems with Enum cases https://github.com/tiangolo/typer/discussions/570", - strict=True, -) def test_CO_vs_Co(): app = Typer() From 2e430459aa520dc12bdf49d577c47986061c9139 Mon Sep 17 00:00:00 2001 From: Alvaro Valdebenito Date: Mon, 20 Mar 2023 15:52:15 +0100 Subject: [PATCH 3/5] Revert "skip CLI options that will confuse typer" This reverts commit c575b1002002a6a4b3de03bb2bbb92f244ee53ec. --- airbase/cli.py | 8 ++++---- tests/test_cli.py | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/airbase/cli.py b/airbase/cli.py index b775a22..ca61a9a 100644 --- a/airbase/cli.py +++ b/airbase/cli.py @@ -10,7 +10,10 @@ from . import __version__ from .airbase import AirbaseClient -main = typer.Typer(no_args_is_help=True, add_completion=False) +main = typer.Typer( + no_args_is_help=True, + add_completion=False, +) client = AirbaseClient() @@ -30,9 +33,6 @@ class Pollutant(str, Enum): Pollutant = vars() for poll in client._pollutants_ids: - if poll.upper() in set(Pollutant): - # skipp options that will confuse typer - continue Pollutant[poll] = poll def __str__(self) -> str: diff --git a/tests/test_cli.py b/tests/test_cli.py index e7ad2a5..9fccac8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,9 +25,13 @@ def test_version(options: str): result = runner.invoke(main, options.split()) assert result.exit_code == 0 assert "airbase" in result.output - assert str(__version__) in result.output + assert __version__ in result.output +@pytest.mark.xfail( + reason="typer has problems with Enum cases https://github.com/tiangolo/typer/discussions/570", + strict=True, +) def test_CO_vs_Co(): app = Typer() From dfa2bb1bc8d8b187ea97f8ee3d2e5ce3759a4765 Mon Sep 17 00:00:00 2001 From: Alvaro Valdebenito Date: Mon, 20 Mar 2023 16:22:43 +0100 Subject: [PATCH 4/5] use click to define CLI sub cmd --- airbase/cli.py | 188 +++++++++++++++++++++++----------- tests/integration/test_cli.py | 9 +- tests/test_cli.py | 4 +- 3 files changed, 135 insertions(+), 66 deletions(-) diff --git a/airbase/cli.py b/airbase/cli.py index ca61a9a..f58b7ed 100644 --- a/airbase/cli.py +++ b/airbase/cli.py @@ -3,17 +3,14 @@ from datetime import date from enum import Enum from pathlib import Path -from typing import List +import click import typer from . import __version__ from .airbase import AirbaseClient -main = typer.Typer( - no_args_is_help=True, - add_completion=False, -) +app = typer.Typer(no_args_is_help=True, add_completion=False) client = AirbaseClient() @@ -47,7 +44,7 @@ def version_callback(value: bool): raise typer.Exit() -@main.callback() +@app.callback() def root_options( version: bool = typer.Option( False, @@ -60,18 +57,83 @@ def root_options( """Download Air Quality Data from the European Environment Agency (EEA)""" -@main.command() +countries = click.option( + "-c", + "--country", + "countries", + type=click.Choice(Country), # type:ignore [arg-type] + multiple=True, +) +country = click.argument( + "country", + type=click.Choice(Country), # type:ignore [arg-type] +) + +pollutants = click.option( + "-p", + "--pollutant", + "pollutants", + type=click.Choice(Pollutant), # type:ignore [arg-type] + multiple=True, +) +pollutant = click.argument( + "pollutant", + type=click.Choice(Pollutant), # type:ignore [arg-type] +) + + +path = click.option( + "--path", + default="data", + type=click.Path(exists=True, dir_okay=True, writable=True), +) +year = click.option("--year", default=date.today().year, type=int) +overwrite = click.option( + "-O", + "--overwrite", + is_flag=True, + help="Re-download existing files.", +) +quiet = click.option( + "-q", + "--quiet", + is_flag=True, + help="No progress-bar.", +) + + +def _download( + countries: list[Country], + pollutants: list[Pollutant], + path: Path, + year: int, + overwrite: bool, + quiet: bool, +): + request = client.request( + countries or None, # type:ignore[arg-type] + pollutants or None, # type:ignore[arg-type] + year_from=str(year), + year_to=str(year), + verbose=not quiet, + ) + request.download_to_directory(path, skip_existing=not overwrite) + + +@click.command() +@countries +@pollutants +@path +@year +@overwrite +@quiet def download( - countries: List[Country] = typer.Option([], "--country", "-c"), - pollutants: List[Pollutant] = typer.Option([], "--pollutant", "-p"), - path: Path = typer.Option( - "data", exists=True, dir_okay=True, writable=True - ), - year: int = typer.Option(date.today().year), - overwrite: bool = typer.Option( - False, "--overwrite", "-O", help="Re-download existing files." - ), - quiet: bool = typer.Option(False, "--quiet", "-q", help="No progress-bar."), + countries: list[Country], + pollutants: list[Pollutant], + path: Path, + year: int, + overwrite: bool, + quiet: bool, ): """Download all pollutants for all countries @@ -82,15 +144,7 @@ def download( - download only SO2, PM10 and PM2.5 observations airbase download -p SO2 -p PM10 -p PM2.5 """ - - request = client.request( - countries or None, # type:ignore[arg-type] - pollutants or None, # type:ignore[arg-type] - year_from=str(year), - year_to=str(year), - verbose=not quiet, - ) - request.download_to_directory(path, skip_existing=not overwrite) + _download(countries, pollutants, path, year, overwrite, quiet) def deprecation_message(old: str, new: str): # pragma: no cover @@ -101,55 +155,69 @@ def deprecation_message(old: str, new: str): # pragma: no cover ) -@main.command(name="all") +@click.command() +@countries +@pollutants +@path +@year +@overwrite +@quiet def download_all( - countries: List[Country] = typer.Option([], "--country", "-c"), - pollutants: List[Pollutant] = typer.Option([], "--pollutant", "-p"), - path: Path = typer.Option( - "data", exists=True, dir_okay=True, writable=True - ), - year: int = typer.Option(date.today().year), - overwrite: bool = typer.Option( - False, "--overwrite", "-O", help="Re-download existing files." - ), - quiet: bool = typer.Option(False, "--quiet", "-q", help="No progress-bar."), + countries: list[Country], + pollutants: list[Pollutant], + path: Path, + year: int, + overwrite: bool, + quiet: bool, ): # pragma: no cover """Download all pollutants for all countries (deprecated)""" deprecation_message("all", "download") - download(countries, pollutants, path, year, overwrite, quiet) + _download(countries, pollutants, path, year, overwrite, quiet) -@main.command(name="country") +@click.command() +@country +@pollutants +@path +@year +@overwrite +@quiet def download_country( country: Country, - pollutants: List[Pollutant] = typer.Option([], "--pollutant", "-p"), - path: Path = typer.Option( - "data", exists=True, dir_okay=True, writable=True - ), - year: int = typer.Option(date.today().year), - overwrite: bool = typer.Option( - False, "--overwrite", "-O", help="Re-download existing files." - ), - quiet: bool = typer.Option(False, "--quiet", "-q", help="No progress-bar."), + pollutants: list[Pollutant], + path: Path, + year: int, + overwrite: bool, + quiet: bool, ): # pragma: no cover """Download specific pollutants for one country (deprecated)""" deprecation_message("country", "download") - download([country], pollutants, path, year, overwrite, quiet) + _download([country], pollutants, path, year, overwrite, quiet) -@main.command(name="pollutant") +@click.command() +@pollutant +@countries +@path +@year +@overwrite +@quiet def download_pollutant( pollutant: Pollutant, - countries: List[Country] = typer.Option([], "--country", "-c"), - path: Path = typer.Option( - "data", exists=True, dir_okay=True, writable=True - ), - year: int = typer.Option(date.today().year), - overwrite: bool = typer.Option( - False, "--overwrite", "-O", help="Re-download existing files." - ), - quiet: bool = typer.Option(False, "--quiet", "-q", help="No progress-bar."), + countries: list[Country], + path: Path, + year: int, + overwrite: bool, + quiet: bool, ): # pragma: no cover """Download specific countries for one pollutant (deprecated)""" deprecation_message("pollutant", "download") - download(countries, [pollutant], path, year, overwrite, quiet) + _download(countries, [pollutant], path, year, overwrite, quiet) + + +# click object +main: click.Group = typer.main.get_command(app) # type:ignore [assignment] +main.add_command(download, "download") +main.add_command(download_all, "all") +main.add_command(download_country, "country") +main.add_command(download_pollutant, "pollutant") diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index f23dd60..b830d04 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -1,6 +1,6 @@ from pathlib import Path -from typer.testing import CliRunner +from click.testing import CliRunner from airbase.cli import main @@ -10,8 +10,9 @@ def test_download(tmp_path: Path): country, year, pollutant, id = "NO", 2021, "NO2", 8 options = f"download --quiet --country {country} --pollutant {pollutant} --year {year} --path {tmp_path}" - result = runner.invoke(main, options.split()) - assert result.exit_code == 0 + with runner.isolated_filesystem(temp_dir=tmp_path): + result = runner.invoke(main, options.split()) + assert result.exit_code == 0 + files = tmp_path.glob(f"{country}_{id}_*_{year}_timeseries.csv") - files = tmp_path.glob(f"{country}_{id}_*_{year}_timeseries.csv") assert list(files) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9fccac8..dac9d6c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,8 +1,8 @@ from __future__ import annotations import pytest +from click.testing import CliRunner from typer import Typer -from typer.testing import CliRunner from airbase import __version__ from airbase.cli import Country, Pollutant, main @@ -25,7 +25,7 @@ def test_version(options: str): result = runner.invoke(main, options.split()) assert result.exit_code == 0 assert "airbase" in result.output - assert __version__ in result.output + assert str(__version__) in result.output @pytest.mark.xfail( From ddd521d37131dce24122a42c97e1ea754a9d0d71 Mon Sep 17 00:00:00 2001 From: Alvaro Valdebenito Date: Thu, 13 Apr 2023 11:44:31 +0200 Subject: [PATCH 5/5] CLI with click --- airbase/cli.py | 54 ++++++++++++----------------------- setup.cfg | 2 +- tests/integration/test_cli.py | 13 +++++++-- tests/test_cli.py | 19 ------------ 4 files changed, 31 insertions(+), 57 deletions(-) diff --git a/airbase/cli.py b/airbase/cli.py index f58b7ed..e4b2807 100644 --- a/airbase/cli.py +++ b/airbase/cli.py @@ -1,16 +1,15 @@ from __future__ import annotations +import sys from datetime import date from enum import Enum from pathlib import Path import click -import typer from . import __version__ from .airbase import AirbaseClient -app = typer.Typer(no_args_is_help=True, add_completion=False) client = AirbaseClient() @@ -36,25 +35,18 @@ def __str__(self) -> str: return self.name -def version_callback(value: bool): - if not value: - return - - typer.echo(f"{__package__} v{__version__}") - raise typer.Exit() - - -@app.callback() -def root_options( - version: bool = typer.Option( - False, - "--version", - "-V", - callback=version_callback, - help=f"Show {__package__} version and exit.", - ), -): +@click.group(invoke_without_command=True, no_args_is_help=True) +@click.option( + "--version", + "-V", + is_flag=True, + help=f"Show {__package__} version and exit.", +) +def main(version: bool): """Download Air Quality Data from the European Environment Agency (EEA)""" + if version: + click.echo(f"{__package__} v{__version__}") + sys.exit(0) countries = click.option( @@ -120,7 +112,7 @@ def _download( request.download_to_directory(path, skip_existing=not overwrite) -@click.command() +@main.command() @countries @pollutants @path @@ -148,14 +140,14 @@ def download( def deprecation_message(old: str, new: str): # pragma: no cover - old = typer.style(f"{__package__} {old}", fg=typer.colors.RED, bold=True) - new = typer.style(f"{__package__} {new}", fg=typer.colors.GREEN, bold=True) - typer.echo( + old = click.style(f"{__package__} {old}", fg="red", bold=True) + new = click.style(f"{__package__} {new}", fg="green", bold=True) + click.echo( f"{old} has been deprecated and will be removed on v1. Use {new} all instead.", ) -@click.command() +@main.command(name="all") @countries @pollutants @path @@ -175,7 +167,7 @@ def download_all( _download(countries, pollutants, path, year, overwrite, quiet) -@click.command() +@main.command(name="country") @country @pollutants @path @@ -195,7 +187,7 @@ def download_country( _download([country], pollutants, path, year, overwrite, quiet) -@click.command() +@main.command(name="pollutant") @pollutant @countries @path @@ -213,11 +205,3 @@ def download_pollutant( """Download specific countries for one pollutant (deprecated)""" deprecation_message("pollutant", "download") _download(countries, [pollutant], path, year, overwrite, quiet) - - -# click object -main: click.Group = typer.main.get_command(app) # type:ignore [assignment] -main.add_command(download, "download") -main.add_command(download_all, "all") -main.add_command(download_country, "country") -main.add_command(download_pollutant, "pollutant") diff --git a/setup.cfg b/setup.cfg index 66d0440..f867f0b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -27,7 +27,7 @@ install_requires = importlib_resources; python_version < "3.11" tqdm typing_extensions; python_version < "3.8" - typer + click packages = find: include_package_data = True diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index b830d04..2f6f96d 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -1,5 +1,6 @@ from pathlib import Path +import pytest from click.testing import CliRunner from airbase.cli import main @@ -7,8 +8,16 @@ runner = CliRunner() -def test_download(tmp_path: Path): - country, year, pollutant, id = "NO", 2021, "NO2", 8 +@pytest.mark.parametrize( + "country,year,pollutant,id", + ( + pytest.param("NO", 2021, "NO2", 8, id="NO2"), + pytest.param("NO", 2021, "CO", 10, id="CO"), + ), +) +def test_download( + country: str, year: int, pollutant: str, id: int, tmp_path: Path +): options = f"download --quiet --country {country} --pollutant {pollutant} --year {year} --path {tmp_path}" with runner.isolated_filesystem(temp_dir=tmp_path): result = runner.invoke(main, options.split()) diff --git a/tests/test_cli.py b/tests/test_cli.py index dac9d6c..c132ad0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2,7 +2,6 @@ import pytest from click.testing import CliRunner -from typer import Typer from airbase import __version__ from airbase.cli import Country, Pollutant, main @@ -26,21 +25,3 @@ def test_version(options: str): assert result.exit_code == 0 assert "airbase" in result.output assert str(__version__) in result.output - - -@pytest.mark.xfail( - reason="typer has problems with Enum cases https://github.com/tiangolo/typer/discussions/570", - strict=True, -) -def test_CO_vs_Co(): - app = Typer() - - pollutants = set() - - @app.command() - def CO_vs_Co(poll: Pollutant): - pollutants.add(poll) - - result = runner.invoke(app, ["CO"]) - assert result.exit_code == 0 - assert pollutants == {Pollutant.CO}