From de47b20a3c9105f279dd6bdd63c04429fc480a2d Mon Sep 17 00:00:00 2001 From: Dmitry Shemetov Date: Wed, 7 Aug 2024 18:03:04 -0700 Subject: [PATCH] repo+lint: switch utilities * switch to Makefile instead of invoke for tasks (better env handling) * switch from black to ruff for linting and formatting * run through ruff linting and formatting * remove aiohttp, black, coverage, invoke, pre-commit, wheel from dependencies * update ci, README --- .github/workflows/ci.yml | 16 +-- .github/workflows/release_helper.yml | 14 +-- .gitignore | 2 +- .pre-commit-config.yaml | 10 -- Makefile | 50 +++++++++ README.md | 27 ++--- epidatpy/_covidcast.py | 23 ++-- epidatpy/_endpoints.py | 9 +- epidatpy/_model.py | 54 ++++----- epidatpy/_parse.py | 4 +- pyproject.toml | 20 ++-- tasks.py | 161 --------------------------- tests/test_epidata_calls.py | 10 +- 13 files changed, 122 insertions(+), 278 deletions(-) delete mode 100644 .pre-commit-config.yaml create mode 100644 Makefile delete mode 100644 tasks.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c459743..a52e1e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,22 +18,16 @@ jobs: cache: "pip" - name: Install Dependencies run: | - python -m venv venv - source venv/bin/activate - pip install -e ".[dev]" + make install - name: Check Formatting run: | - source venv/bin/activate - inv lint-black + make lint_ruff - name: Check Linting run: | - source venv/bin/activate - inv lint-pylint + make lint_pylint - name: Check Types run: | - source venv/bin/activate - inv lint-mypy + make lint_mypy - name: Test run: | - source venv/bin/activate - inv test + make test diff --git a/.github/workflows/release_helper.yml b/.github/workflows/release_helper.yml index 4e6291b..a2fb61b 100644 --- a/.github/workflows/release_helper.yml +++ b/.github/workflows/release_helper.yml @@ -52,16 +52,13 @@ jobs: python-version: 3.8 - name: Install build dependencies run: | - python -m pip install --upgrade pip - pip install -e ".[dev]" + make install - name: Linting run: | - . venv/bin/activate - inv lint + make lint - name: Testing run: | - . venv/bin/activate - inv test + make test build: needs: [create_release, lint] @@ -75,11 +72,10 @@ jobs: python-version: 3.8 - name: Install build dependencies run: | - python -m pip install --upgrade pip - pip install -e ".[dev]" + make install - name: Build run: | - inv dist + make dist release_package: needs: [create_release, lint] diff --git a/.gitignore b/.gitignore index aa58631..f7c6b05 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,4 @@ __pycache__ dist/ build/ docs/_build -venv/ \ No newline at end of file +env/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml deleted file mode 100644 index 92529d6..0000000 --- a/.pre-commit-config.yaml +++ /dev/null @@ -1,10 +0,0 @@ -repos: -- repo: https://github.com/astral-sh/ruff-pre-commit - # Ruff version. - rev: v0.5.1 - hooks: - # Run the linter. - - id: ruff - args: [--fix] - # Run the formatter. - - id: ruff-format diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..e5f46e4 --- /dev/null +++ b/Makefile @@ -0,0 +1,50 @@ +.PHONY = venv, lint, test, clean, release + +venv: + python3.8 -m venv env + +install: venv + env/bin/python -m pip install --upgrade pip + env/bin/pip install -e ".[dev]" + +lint_ruff: + env/bin/ruff check epidatpy tests + +lint_mypy: + env/bin/mypy epidatpy tests + +lint_pylint: + env/bin/pylint epidatpy tests + +lint: lint_ruff lint_mypy lint_pylint + +format: + env/bin/ruff format epidatpy tests + +test: + env/bin/pytest . + +docs: + env/bin/sphinx-build -b html docs docs/_build + python -m webbrowser -t "docs/_build/index.html" + +clean_docs: + rm -rf docs/_build + +clean_build: + rm -rf build dist .eggs + find . -name '*.egg-info' -exec rm -rf {} + + find . -name '*.egg' -exec rm -f {} + + +clean_python: + find . -name '*.pyc' -exec rm -f {} + + find . -name '*.pyo' -exec rm -f {} + + find . -name '__pycache__' -exec rm -fr {} + + +clean: clean_docs clean_build clean_python + +release: clean lint test + env/bin/python -m build --sdist --wheel + +upload: release + env/bin/twine upload dist/* \ No newline at end of file diff --git a/README.md b/README.md index b8a5107..a366317 100644 --- a/README.md +++ b/README.md @@ -22,26 +22,17 @@ TODO ## Development -Prepare virtual environment and install dependencies +The following commands are available for developers: ```sh -python -m venv venv -source ./venv/bin/activate -pip install -e ".[dev]" -``` - -### Common Commands - -```sh -source ./venv/bin/activate -inv format # format code -inv lint # check linting -inv docs # build docs -inv test # run unit tests -inv coverage # run unit tests with coverage -inv clean # clean build artifacts -inv dist # build distribution packages -inv release # upload the current version to pypi +make install # setup venv, install dependencies and local package +make test # run unit tests +make format # format code +make lint # check linting +make docs # build docs +make dist # build distribution packages +make release # upload the current version to pypi +make clean # clean build and docs artifacts ``` ### Release Process diff --git a/epidatpy/_covidcast.py b/epidatpy/_covidcast.py index 43e2b99..4b6557e 100644 --- a/epidatpy/_covidcast.py +++ b/epidatpy/_covidcast.py @@ -33,8 +33,7 @@ @dataclass class WebLink: - """ - represents a web link + """represents a web link """ alt: str @@ -43,8 +42,7 @@ class WebLink: @dataclass class DataSignalGeoStatistics: - """ - COVIDcast signal statistics + """COVIDcast signal statistics """ min: float @@ -64,7 +62,11 @@ def define_covidcast_fields() -> List[EpidataFieldInfo]: EpidataFieldInfo("signal", EpidataFieldType.text), EpidataFieldInfo("geo_type", EpidataFieldType.categorical, categories=list(get_args(GeoType))), EpidataFieldInfo("geo_value", EpidataFieldType.text), - EpidataFieldInfo("time_type", EpidataFieldType.categorical, categories=list(get_args(TimeType))), + EpidataFieldInfo( + "time_type", + EpidataFieldType.categorical, + categories=list(get_args(TimeType)), + ), EpidataFieldInfo("time_value", EpidataFieldType.date_or_epiweek), EpidataFieldInfo("issue", EpidataFieldType.date), EpidataFieldInfo("lag", EpidataFieldType.int), @@ -80,8 +82,7 @@ def define_covidcast_fields() -> List[EpidataFieldInfo]: @dataclass class DataSignal(Generic[CALL_TYPE]): - """ - represents a COVIDcast data signal + """represents a COVIDcast data signal """ _create_call: Callable[[Mapping[str, Optional[EpiRangeParam]]], CALL_TYPE] @@ -160,7 +161,7 @@ def call( lag: Optional[int] = None, ) -> CALL_TYPE: """Fetch Delphi's COVID-19 Surveillance Streams""" - if any((v is None for v in (geo_type, geo_values, time_values))): + if any(v is None for v in (geo_type, geo_values, time_values)): raise InvalidArgumentException("`geo_type`, `time_values`, and `geo_values` are all required") if issues is not None and lag is not None: raise InvalidArgumentException("`issues` and `lag` are mutually exclusive") @@ -194,8 +195,7 @@ def __call__( @dataclass class DataSource(Generic[CALL_TYPE]): - """ - represents a COVIDcast data source + """represents a COVIDcast data source """ _create_call: InitVar[Callable[[Mapping[str, Optional[EpiRangeParam]]], CALL_TYPE]] @@ -247,8 +247,7 @@ def signal_df(self) -> DataFrame: @dataclass class CovidcastDataSources(Generic[CALL_TYPE]): - """ - COVIDcast data source helper. + """COVIDcast data source helper. """ sources: Sequence[DataSource[CALL_TYPE]] diff --git a/epidatpy/_endpoints.py b/epidatpy/_endpoints.py index fd308e1..3646c1a 100644 --- a/epidatpy/_endpoints.py +++ b/epidatpy/_endpoints.py @@ -36,8 +36,7 @@ def get_wildcard_equivalent_dates(time_value: EpiRangeParam, time_type: Literal[ class AEpiDataEndpoints(ABC, Generic[CALL_TYPE]): - """ - epidata endpoint list and fetcher + """epidata endpoint list and fetcher """ @abstractmethod @@ -87,8 +86,7 @@ def pub_covid_hosp_facility_lookup( fips_code: Optional[str] = None, ) -> CALL_TYPE: """Lookup COVID hospitalization facility identifiers.""" - - if all((v is None for v in (state, ccn, city, zip, fips_code))): + if all(v is None for v in (state, ccn, city, zip, fips_code)): raise InvalidArgumentException("one of `state`, `ccn`, `city`, `zip`, or `fips_code` is required") return self._create_call( @@ -121,7 +119,6 @@ def pub_covid_hosp_facility( publication_dates: Optional[EpiRangeParam] = None, ) -> CALL_TYPE: """Fetch COVID hospitalization data for specific facilities.""" - collection_weeks = get_wildcard_equivalent_dates(collection_weeks, "day") # Confusingly, the endpoint expects `collection_weeks` to be in day format, @@ -271,7 +268,6 @@ def pub_covid_hosp_state_timeseries( as_of: Union[None, int, str] = None, ) -> CALL_TYPE: """Fetch COVID hospitalization data.""" - if issues is not None and as_of is not None: raise InvalidArgumentException("`issues` and `as_of` are mutually exclusive") @@ -481,7 +477,6 @@ def pub_covidcast( def pub_delphi(self, system: str, epiweek: Union[int, str]) -> CALL_TYPE: """Fetch Delphi's forecast.""" - return self._create_call( "delphi/", {"system": system, "epiweek": epiweek}, diff --git a/epidatpy/_model.py b/epidatpy/_model.py index 2d426fe..83eb968 100644 --- a/epidatpy/_model.py +++ b/epidatpy/_model.py @@ -35,10 +35,17 @@ StringParam = Union[str, Sequence[str]] IntParam = Union[int, Sequence[int]] ParamType = Union[StringParam, IntParam, EpiRangeParam] -EpiDataResponse = TypedDict("EpiDataResponse", {"result": int, "message": str, "epidata": List}) CALL_TYPE = TypeVar("CALL_TYPE") +class EpiDataResponse(TypedDict): + """response from the API""" + + result: int + message: str + epidata: List + + def format_date(d: EpiDateLike) -> str: if isinstance(d, date): # YYYYMMDD @@ -70,9 +77,7 @@ def format_list(values: EpiRangeParam) -> str: class EpiRange: - """ - Range object for dates/epiweeks - """ + """Range object for dates/epiweeks""" def __init__(self, start: EpiDateLike, end: EpiDateLike) -> None: # check if types are correct @@ -91,21 +96,15 @@ def __str__(self) -> str: class InvalidArgumentException(Exception): - """ - exception for an invalid argument - """ + """exception for an invalid argument""" class OnlySupportsClassicFormatException(Exception): - """ - the endpoint only supports the classic message format, due to an non-standard behavior - """ + """the endpoint only supports the classic message format, due to an non-standard behavior""" class EpidataFieldType(Enum): - """ - field type - """ + """field type""" text = 0 int = 1 @@ -119,9 +118,7 @@ class EpidataFieldType(Enum): @dataclass class EpidataFieldInfo: - """ - meta data information about an return field - """ + """meta data information about an return field""" name: Final[str] = "" type: Final[EpidataFieldType] = EpidataFieldType.text @@ -137,9 +134,7 @@ def add_endpoint_to_url(url: str, endpoint: str) -> str: class AEpiDataCall: - """ - base epidata call class - """ + """base epidata call class""" _base_url: Final[str] _endpoint: Final[str] @@ -167,8 +162,11 @@ def __init__( self.meta_by_name = {k.name: k for k in self.meta} # Set the use_cache value from the constructor if present. # Otherwise check the USE_EPIDATPY_CACHE variable, accepting various "truthy" values. - self.use_cache = use_cache if use_cache is not None \ - else (environ.get("USE_EPIDATPY_CACHE", "").lower() in ['true', 't', '1']) + self.use_cache = ( + use_cache + if use_cache is not None + else (environ.get("USE_EPIDATPY_CACHE", "").lower() in ["true", "t", "1"]) + ) # Set cache_max_age_days from the constructor, fall back to environment variable. if cache_max_age_days: self.cache_max_age_days = cache_max_age_days @@ -176,7 +174,7 @@ def __init__( env_days = environ.get("EPIDATPY_CACHE_MAX_AGE_DAYS", "7") if env_days.isdigit(): self.cache_max_age_days = int(env_days) - else: # handle string / negative / invalid enviromment variable + else: # handle string / negative / invalid enviromment variable self.cache_max_age_days = 7 def _verify_parameters(self) -> None: @@ -187,9 +185,7 @@ def _formatted_parameters( self, fields: Optional[Sequence[str]] = None, ) -> Mapping[str, str]: - """ - format this call into a [URL, Params] tuple - """ + """Format this call into a [URL, Params] tuple""" all_params = dict(self._params) if fields: all_params["fields"] = fields @@ -199,9 +195,7 @@ def request_arguments( self, fields: Optional[Sequence[str]] = None, ) -> Tuple[str, Mapping[str, str]]: - """ - format this call into a [URL, Params] tuple - """ + """Format this call into a [URL, Params] tuple""" formatted_params = self._formatted_parameters(fields) full_url = add_endpoint_to_url(self._base_url, self._endpoint) return full_url, formatted_params @@ -210,9 +204,7 @@ def request_url( self, fields: Optional[Sequence[str]] = None, ) -> str: - """ - format this call into a full HTTP request url with encoded parameters - """ + """Format this call into a full HTTP request url with encoded parameters""" self._verify_parameters() u, p = self.request_arguments(fields) query = urlencode(p) diff --git a/epidatpy/_parse.py b/epidatpy/_parse.py index 2f7b10c..5af33b6 100644 --- a/epidatpy/_parse.py +++ b/epidatpy/_parse.py @@ -71,7 +71,9 @@ def parse_user_date_or_week( raise ValueError(f"Cannot parse date or week from {value}") -def fields_to_predicate(fields: Optional[Sequence[str]] = None) -> Callable[[str], bool]: +def fields_to_predicate( + fields: Optional[Sequence[str]] = None, +) -> Callable[[str], bool]: if not fields: return lambda _: True to_include: Set[str] = set() diff --git a/pyproject.toml b/pyproject.toml index 8215c10..8321b5c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,7 +30,6 @@ classifiers = [ ] requires-python = ">=3.8" dependencies = [ - "aiohttp", "appdirs", "diskcache", "epiweeks>=2.1", @@ -41,21 +40,16 @@ dependencies = [ [project.optional-dependencies] dev = [ - "black", - "coverage", - "invoke", "mypy", - "pre-commit", "pylint", "pytest", "recommonmark", + "ruff", "sphinx_rtd_theme", "sphinx-autodoc-typehints", "sphinx", "twine", "types-requests", - "watchdog", - "wheel", ] [project.urls] @@ -63,12 +57,14 @@ homepage = "https://github.com/cmu-delphi/epidatpy" repository = "https://github.com/cmu-delphi/epidatpy" -[tool.black] -line-length = 120 -target-version = ['py38'] - [tool.ruff] -lint.extend-select = ["I"] +line-length = 120 +format.docstring-code-format = true +format.docstring-code-line-length = "dynamic" +lint.extend-select = [ + "I", # isort + "UP", # pyupgrade +] [tool.pylint] max-line-length = 120 diff --git a/tasks.py b/tasks.py deleted file mode 100644 index afeb455..0000000 --- a/tasks.py +++ /dev/null @@ -1,161 +0,0 @@ -""" -Tasks for maintaining the project. - -Execute 'invoke --list' for guidance on using Invoke -""" - -import shutil -import webbrowser -from pathlib import Path - -from invoke import task - -ROOT_DIR = Path(__file__).parent -TEST_DIR = ROOT_DIR.joinpath("tests") -SOURCE_DIR = ROOT_DIR.joinpath("epidatpy") -TOX_DIR = ROOT_DIR.joinpath(".tox") -COVERAGE_FILE = ROOT_DIR.joinpath(".coverage") -COVERAGE_DIR = ROOT_DIR.joinpath("htmlcov") -COVERAGE_REPORT = COVERAGE_DIR.joinpath("index.html") -DOCS_DIR = ROOT_DIR.joinpath("docs") -DOCS_BUILD_DIR = DOCS_DIR.joinpath("_build") -DOCS_INDEX = DOCS_BUILD_DIR.joinpath("index.html") -PYTHON_DIRS = [str(d) for d in [SOURCE_DIR, TEST_DIR]] -JOINED_PYTHON_DIRS = " ".join(PYTHON_DIRS) - - -def _delete_file(file: Path) -> None: - try: - file.unlink(missing_ok=True) - except TypeError: - # missing_ok argument added in 3.8 - try: - file.unlink() - except FileNotFoundError: - pass - - -@task() -def format(c): # pylint: redefined-builtin - """ - Format code - """ - c.run("black --config pyproject.toml {}".format(JOINED_PYTHON_DIRS)) - - -@task -def lint_black(c): - c.run("black --check --config pyproject.toml {}".format(JOINED_PYTHON_DIRS)) - - -@task -def lint_pylint(c): - c.run("pylint {}".format(JOINED_PYTHON_DIRS)) - - -@task -def lint_mypy(c): - c.run("mypy --ignore-missing-imports {}".format(JOINED_PYTHON_DIRS)) - - -@task(pre=[lint_black, lint_pylint, lint_mypy]) -def lint(c): # pylint: disable=unused-argument - """ - Lint code - """ - - -@task -def test(c): - """ - Run tests - """ - c.run("pytest {}".format(TEST_DIR)) - - -@task(help={"publish": "Publish the result via coveralls"}) -def coverage(c, publish=False): - """ - Create coverage report - """ - c.run("coverage run --source {} -m pytest".format(TEST_DIR)) - c.run("coverage report") - if publish: - # Publish the results via coveralls - c.run("coveralls") - else: - # Build a local report - c.run("coverage html") - webbrowser.open(COVERAGE_REPORT.as_uri()) - - -@task -def docs(c): # type: ignore - """ - Generate documentation - """ - c.run("sphinx-build -b html {} {}".format(DOCS_DIR, DOCS_BUILD_DIR)) - webbrowser.open(DOCS_INDEX.as_uri()) - - -@task -def clean_docs(c): # pylint: disable=unused-argument - """ - Clean up files from documentation builds - """ - shutil.rmtree(DOCS_BUILD_DIR, ignore_errors=True) - - -@task -def clean_build(c): - """ - Clean up files from package building - """ - shutil.rmtree("build", ignore_errors=True) - shutil.rmtree("dist", ignore_errors=True) - shutil.rmtree(".eggs", ignore_errors=True) - c.run("find . -name '*.egg-info' -exec rm -fr {} +") - c.run("find . -name '*.egg' -exec rm -f {} +") - - -@task -def clean_python(c): - """ - Clean up python file artifacts - """ - c.run("find . -name '*.pyc' -exec rm -f {} +") - c.run("find . -name '*.pyo' -exec rm -f {} +") - c.run("find . -name '__pycache__' -exec rm -fr {} +") - - -@task -def clean_tests(c): # pylint: disable=unused-argument - """ - Clean up files from testing - """ - _delete_file(COVERAGE_FILE) - shutil.rmtree(TOX_DIR, ignore_errors=True) - shutil.rmtree(COVERAGE_DIR, ignore_errors=True) - - -@task(pre=[clean_build, clean_python, clean_tests, clean_docs]) -def clean(c): # pylint: disable=unused-argument - """ - Runs all clean sub-tasks - """ - - -@task(clean) -def dist(c): - """ - Build source and wheel packages - """ - c.run("python -m build --sdist --wheel") - - -@task(pre=[clean, dist]) -def release(c): - """ - Make a release of the python package to pypi - """ - c.run("twine upload dist/*") diff --git a/tests/test_epidata_calls.py b/tests/test_epidata_calls.py index 13cd9e1..b827225 100644 --- a/tests/test_epidata_calls.py +++ b/tests/test_epidata_calls.py @@ -320,15 +320,15 @@ def test_pub_nidss_flu(self) -> None: assert str(data["ili"].dtype) == "Float64" @pytest.mark.skipif(not secret_norostat, reason="Norostat key not available.") + @pytest.mark.skip(reason="TODO: Need working Norostat query.") def test_pvt_norostat(self) -> None: apicall = EpiDataContext().pvt_norostat(auth=secret_norostat, location="1", epiweeks=201233) data = apicall.df() - # TODO: Need a non-trivial query for Norostat - # assert len(data) > 0 - # assert str(data["release_date"].dtype) == "datetime64[ns]" - # assert str(data["epiweek"].dtype) == "string" - # assert str(data["value"].dtype) == "Int64" + assert len(data) > 0 + assert str(data["release_date"].dtype) == "datetime64[ns]" + assert str(data["epiweek"].dtype) == "string" + assert str(data["value"].dtype) == "Int64" def test_pub_nowcast(self) -> None: apicall = EpiDataContext().pub_nowcast(locations="ca", epiweeks=EpiRange(201201, 201301))