From ad279cf7bc4d1abb4f6076698c178d2bcf3145d0 Mon Sep 17 00:00:00 2001 From: Lukas Heumos Date: Tue, 7 Jan 2025 19:45:55 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20Remove=20lnschema-core=20install?= =?UTF-8?q?ation=20(#44)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: zethson --- .github/workflows/build.yml | 2 +- .pre-commit-config.yaml | 79 ++++++-------- docs/notes/YYYY-MM-DD-my-design-choice.ipynb | 4 +- laminci/__main__.py | 33 +++--- laminci/_db.py | 3 +- laminci/_doc_changes.py | 8 +- laminci/_docs.py | 5 +- laminci/_docs_artifacts.py | 13 ++- laminci/_env.py | 2 +- laminci/_nox_logger.py | 5 +- laminci/_run_notebooks.py | 2 +- laminci/nox.py | 17 ++- pyproject.toml | 107 ++++++++++++++++++- tests/test_package_name.py | 2 +- 14 files changed, 190 insertions(+), 92 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e4ef9bb..c58862f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.8"] # we want to be able to run this on 3.8 + python-version: ["3.12"] timeout-minutes: 10 steps: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8f9281e..1db0008 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,40 +1,21 @@ +fail_fast: false +default_language_version: + python: python3 +default_stages: + - commit + - push +minimum_pre_commit_version: 2.12.0 repos: - - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v3.2.0 - hooks: - - id: trailing-whitespace - - id: end-of-file-fixer - exclude: | - (?x)( - .github/workflows/latest-changes.jinja2 - ) - - id: check-yaml - - id: check-added-large-files - - repo: https://github.com/psf/black - rev: 22.6.0 - hooks: - - id: black-jupyter - - repo: https://github.com/pycqa/flake8 - rev: 4.0.1 + - repo: https://github.com/pre-commit/mirrors-prettier + rev: v4.0.0-alpha.4 hooks: - - id: flake8 - additional_dependencies: - - flake8-black==0.3.3 - - flake8-typing-imports==1.10.0 - language_version: python3 - args: - - --max-line-length=88 - - --ignore=E203,W503,BLK100,TYP001 + - id: prettier exclude: | (?x)( - __init__.py + docs/changelog.md ) - - repo: https://github.com/pre-commit/mirrors-prettier - rev: v2.6.2 - hooks: - - id: prettier - repo: https://github.com/kynan/nbstripout - rev: 0.3.9 + rev: 0.6.1 hooks: - id: nbstripout exclude: | @@ -42,23 +23,29 @@ repos: docs/examples/| docs/notes/ ) - - repo: https://github.com/Lucas-C/pre-commit-hooks - rev: v1.1.9 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.8.6 hooks: - - id: forbid-crlf - - id: remove-crlf - - repo: https://github.com/pre-commit/mirrors-isort - rev: v5.8.0 + - id: ruff + args: [--fix, --exit-non-zero-on-fix, --unsafe-fixes] + - id: ruff-format + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 hooks: - - id: isort - args: ["--profile", "black"] + - id: detect-private-key + - id: check-ast + - id: end-of-file-fixer + exclude: | + (?x)( + .github/workflows/latest-changes.jinja2 + ) + - id: mixed-line-ending + args: [--fix=lf] + - id: trailing-whitespace + - id: check-case-conflict - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.940 + rev: v1.7.1 hooks: - id: mypy - - repo: https://github.com/pycqa/pydocstyle - rev: 6.1.1 - hooks: - - id: pydocstyle - args: # google style + __init__, see http://www.pydocstyle.org/en/stable/error_codes.html - - --ignore=D100,D101,D102,D103,D106,D107,D203,D204,D213,D215,D400,D401,D403,D404,D406,D407,D408,D409,D412,D413 + args: [--no-strict-optional, --ignore-missing-imports] + additional_dependencies: ["types-requests", "types-attrs"] diff --git a/docs/notes/YYYY-MM-DD-my-design-choice.ipynb b/docs/notes/YYYY-MM-DD-my-design-choice.ipynb index 1ad4b0d..a15463f 100644 --- a/docs/notes/YYYY-MM-DD-my-design-choice.ipynb +++ b/docs/notes/YYYY-MM-DD-my-design-choice.ipynb @@ -14,9 +14,7 @@ "id": "97ec6631-8473-4a2d-b488-def921bb83de", "metadata": {}, "outputs": [], - "source": [ - "from nbproject import header" - ] + "source": [] } ], "metadata": { diff --git a/laminci/__main__.py b/laminci/__main__.py index 225c79b..c49fe6c 100644 --- a/laminci/__main__.py +++ b/laminci/__main__.py @@ -8,7 +8,6 @@ import subprocess from pathlib import Path from subprocess import PIPE, run -from typing import Union from packaging.version import Version, parse @@ -35,7 +34,7 @@ def update_readme_version(file_path, new_version): # Read the content of the file - with open(file_path, "r") as file: + with open(file_path) as file: content = file.read() # Use regex to find and replace the version @@ -49,7 +48,7 @@ def update_readme_version(file_path, new_version): def get_last_version_from_tags(): - proc = run(["git", "tag"], universal_newlines=True, stdout=PIPE) + proc = run(["git", "tag"], text=True, stdout=PIPE) tags = proc.stdout.splitlines() newest = "0.0.0" for tag in tags: @@ -64,16 +63,18 @@ def validate_version(version_str: str): if not len(version.release) == 2: raise SystemExit( f"Pre-releases should be of form 0.42a1 or 0.42rc1, yours is {version}" - ) + ) from None else: return None if len(version.release) != 3: - raise SystemExit(f"Version should be of form 0.1.2, yours is {version}") + raise SystemExit( + f"Version should be of form 0.1.2, yours is {version}" + ) from None def publish_github_release( repo_name: str, - version: Union[str, Version], + version: str | Version, release_name: str, body: str = "", draft: bool = False, @@ -132,12 +133,14 @@ def publish_github_release( generate_release_notes=generate_release_notes, ) except GithubException as e: - raise SystemExit(f"Error creating GitHub release using `PyGithub`: {e}") + raise SystemExit( + f"Error creating GitHub release using `PyGithub`: {e}" + ) from None except ImportError: raise SystemExit( "Neither the Github CLI ('gh') nor PyGithub were accessible.\n" "Please install one of the two." - ) + ) from None def main(): @@ -163,15 +166,15 @@ def main(): raise SystemExit( "Please pass a link to the changelog entry via: --changelog" " 'your-link'" - ) + ) from None if Path("./LICENSE").exists() and not args.pypi: raise SystemExit( "ERROR: Did you forget to add the `--pypi` flag? A LICENSE file" " exists and I assume this is an open-source package." - ) + ) from None repo_name = package_name.replace("_", "-") else: - assert Path.cwd().name == "laminhub" + assert Path.cwd().name == "laminhub" # noqa: S101 repo_name = "laminhub" if not (Path.cwd().parent / "laminhub-public").exists(): raise ValueError( @@ -179,7 +182,7 @@ def main(): " directory as laminhub." ) is_laminhub = True - with open("ui/package.json", "r") as file: + with open("ui/package.json") as file: version = json.load(file)["version"] print(f"INFO: You will add this changelog link: {args.changelog}") @@ -203,7 +206,7 @@ def main(): ] for command in commands: print(f"\nrun: {command}") - run(command, shell=True) + run(command, shell=True) # noqa: S602 changelog_link = ( args.changelog @@ -221,7 +224,7 @@ def main(): update_readme_version("../laminhub-public/README.md", version) for command in commands: print(f"\nrun: {command}") - run(command, shell=True, cwd="../laminhub-public") + run(command, shell=True, cwd="../laminhub-public") # noqa: S602 publish_github_release( repo_name="laminlabs/laminhub-public", version=version, @@ -234,7 +237,7 @@ def main(): if args.pypi: command = "flit publish" print(f"\nrun: {command}") - run(command, shell=True) + run(command, shell=True) # noqa: S602 elif args.command == "doc-changes": from ._doc_changes import doc_changes diff --git a/laminci/_db.py b/laminci/_db.py index 12a5499..a9f08d9 100644 --- a/laminci/_db.py +++ b/laminci/_db.py @@ -27,8 +27,7 @@ def setup_local_test_postgres(name: str = "pgtest", version: Optional[str] = Non version = "" process = run( f"docker run --name {name} -e POSTGRES_PASSWORD=pwd" - f" -e POSTGRES_DB={name} -d -p 5432:5432 postgres{version}", # noqa - shell=True, + f" -e POSTGRES_DB={name} -d -p 5432:5432 postgres{version}", ) if process.returncode == 0: logger.info( diff --git a/laminci/_doc_changes.py b/laminci/_doc_changes.py index fd5fb33..d8ba11f 100644 --- a/laminci/_doc_changes.py +++ b/laminci/_doc_changes.py @@ -33,13 +33,16 @@ import subprocess import sys from pathlib import Path +from typing import TYPE_CHECKING from github import Github -from github.PullRequest import PullRequest from jinja2 import Template from pydantic import BaseModel, SecretStr from pydantic_settings import BaseSettings +if TYPE_CHECKING: + from github.PullRequest import PullRequest + class Section(BaseModel): label: str @@ -209,8 +212,7 @@ def generate_content( new_release_content = updated_content new_content = ( - f"{pre_header_content}\n\n{new_release_content}\n\n{post_release_content}" - .strip() + f"{pre_header_content}\n\n{new_release_content}\n\n{post_release_content}".strip() + "\n" ) return new_content diff --git a/laminci/_docs.py b/laminci/_docs.py index 6e55378..70ed58f 100644 --- a/laminci/_docs.py +++ b/laminci/_docs.py @@ -1,5 +1,6 @@ import os import shutil +from pathlib import Path from ._env import load_project_yaml @@ -9,7 +10,7 @@ def move_built_docs_to_slash_project_slug(): return yaml = load_project_yaml() shutil.move("_build/html", "_build/html_tmp") - os.makedirs("_build/html") + Path.mkdir("_build/html", parents=True) shutil.move("_build/html_tmp", f"_build/html/{yaml['project_slug']}") @@ -18,5 +19,5 @@ def move_built_docs_to_docs_slash_project_slug(): return yaml = load_project_yaml() shutil.move("_build/html", f"_build/{yaml['project_slug']}") - os.makedirs("_build/html/docs") + Path.mkdir("_build/html/docs", parents=True) shutil.move(f"_build/{yaml['project_slug']}", "_build/html/docs") diff --git a/laminci/_docs_artifacts.py b/laminci/_docs_artifacts.py index a363440..00cb26b 100644 --- a/laminci/_docs_artifacts.py +++ b/laminci/_docs_artifacts.py @@ -19,14 +19,16 @@ def zip_docs_dir(zip_filename: str) -> None: def zip_docs(): repo_name = Path.cwd().name - assert "." not in repo_name # doesn't have a weird suffix - assert Path(".git/").exists() # is git repo - assert repo_name.lower() == repo_name # is all lower-case + assert "." not in repo_name # doesn't have a weird suffix # noqa: S101 + assert Path(".git/").exists() # is git repo # noqa: S101 + assert repo_name.lower() == repo_name # is all lower-case # noqa: S101 zip_filename = f"{repo_name}.zip" zip_docs_dir(zip_filename) return repo_name, zip_filename +# Ruff seems to ignore any noqa comments in the following and we therefore disable it briefly +# ruff: noqa def upload_docs_artifact_aws() -> None: repo_name, zip_filename = zip_docs() run( @@ -35,6 +37,9 @@ def upload_docs_artifact_aws() -> None: ) +# ruff: enable + + def upload_docs_artifact_lamindb() -> None: repo_name, zip_filename = zip_docs() @@ -73,5 +78,5 @@ def upload_docs_artifact(aws: bool = False) -> None: upload_docs_artifact_lamindb() except ImportError: - warnings.warn("Fall back to AWS") + warnings.warn("Fall back to AWS", stacklevel=2) upload_docs_artifact_aws() diff --git a/laminci/_env.py b/laminci/_env.py index 241ec8d..e7d2ac1 100644 --- a/laminci/_env.py +++ b/laminci/_env.py @@ -8,7 +8,7 @@ def load_project_yaml(root_directory: Optional[Path] = None) -> dict: yaml_file = Path("lamin-project.yaml") if root_directory is None: - root_directory = Path(".") + root_directory = Path() yaml_file = root_directory / yaml_file with yaml_file.open() as f: d = yaml.safe_load(f) diff --git a/laminci/_nox_logger.py b/laminci/_nox_logger.py index bd5a7e0..ac0c938 100644 --- a/laminci/_nox_logger.py +++ b/laminci/_nox_logger.py @@ -1,11 +1,14 @@ from __future__ import annotations import logging -from collections.abc import Sequence +from typing import TYPE_CHECKING import nox from nox.registry import _REGISTRY, Any, Callable, F, Func, Python, functools +if TYPE_CHECKING: + from collections.abc import Sequence + def session_decorator( func: F | None = None, diff --git a/laminci/_run_notebooks.py b/laminci/_run_notebooks.py index d72dc28..c12393d 100644 --- a/laminci/_run_notebooks.py +++ b/laminci/_run_notebooks.py @@ -7,5 +7,5 @@ def run_notebooks(file_or_folder: str | Path): import nbproject_test path = Path(file_or_folder) - assert path.exists() + assert path.exists() # noqa: S101c nbproject_test.execute_notebooks(path.resolve(), write=True) diff --git a/laminci/nox.py b/laminci/nox.py index 7a72f60..0d2689a 100644 --- a/laminci/nox.py +++ b/laminci/nox.py @@ -1,20 +1,20 @@ import json import os import shlex +from collections.abc import Iterable from pathlib import Path -from typing import Dict, Iterable, Literal, Optional, Union +from typing import Literal, Optional, Union import nox from nox import Session -from . import _nox_logger # noqa, the import silences the logger from ._env import get_package_name SYSTEM = " --system " if os.getenv("CI") else "" nox.options.default_venv_backend = "none" -def _login_lamin_user(user_email: str, env: Optional[Dict[str, str]] = None): +def _login_lamin_user(user_email: str, env: Optional[dict[str, str]] = None): import boto3 import lamindb_setup as ln_setup @@ -37,16 +37,16 @@ def _login_lamin_user(user_email: str, env: Optional[Dict[str, str]] = None): raise NotImplementedError -def login_testuser1(session: Session, env: Optional[Dict[str, str]] = None): +def login_testuser1(session: Session, env: Optional[dict[str, str]] = None): _login_lamin_user("testuser1@lamin.ai", env=env) -def login_testuser2(session: Session, env: Optional[Dict[str, str]] = None): +def login_testuser2(session: Session, env: Optional[dict[str, str]] = None): _login_lamin_user("testuser2@lamin.ai", env=env) def run(session: Session, s: str, **kwargs): - assert (args := shlex.split(s)) + assert (args := shlex.split(s)) # noqa: S101 return session.run(*args, **kwargs) @@ -59,7 +59,7 @@ def run_pre_commit(session: Session): session.run("pre-commit", "run", "--all-files") -def run_pytest(session: Session, coverage: bool = True, env: Optional[Dict] = None): +def run_pytest(session: Session, coverage: bool = True, env: Optional[dict] = None): package_name = get_package_name() coverage_args = ( f"--cov={package_name} --cov-append --cov-report=term-missing".split() @@ -101,7 +101,7 @@ def install_lamindb( if extras == "": extras_str = "" else: - assert "[" not in extras and "]" not in extras + assert "[" not in extras and "]" not in extras # noqa: S101 extras_str = f"[{extras}]" else: extras_str = f"[{','.join(extras)}]" @@ -125,7 +125,6 @@ def install_lamindb( "--system", "--no-deps", "./lamindb/sub/lamindb-setup", - "./lamindb/sub/lnschema-core", "./lamindb/sub/lamin-cli", "./lamindb/sub/bionty", "./lamindb/sub/wetlab", diff --git a/pyproject.toml b/pyproject.toml index b13dede..5ace26d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,9 +42,6 @@ dev = [ [project.scripts] laminci = "laminci.__main__:main" -[tool.black] -preview = true - [tool.pytest.ini_options] testpaths = [ "tests", @@ -54,3 +51,107 @@ testpaths = [ omit = [ "laminci/*", ] + +[tool.ruff] +src = ["src"] +line-length = 88 +lint.select = [ + "F", # Errors detected by Pyflakes + "E", # Error detected by Pycodestyle + "W", # Warning detected by Pycodestyle + "I", # isort + "D", # pydocstyle + "B", # flake8-bugbear + "TID", # flake8-tidy-imports + "C4", # flake8-comprehensions + "BLE", # flake8-blind-except + "UP", # pyupgrade + "RUF100", # Report unused noqa directives + "TCH", # Typing imports + "NPY", # Numpy specific rules + "PTH", # Use pathlib + "S" # Security +] +lint.ignore = [ + # Do not catch blind exception: `Exception` + "BLE001", + # Errors from function calls in argument defaults. These are fine when the result is immutable. + "B008", + # line too long -> we accept long comment lines; black gets rid of long code lines + "E501", + # Do not assign a lambda expression, use a def -> lambda expression assignments are convenient + "E731", + # allow I, O, l as variable names -> I is the identity matrix + "E741", + # Missing docstring in public module + "D100", + # undocumented-public-class + "D101", + # Missing docstring in public method + "D102", + # Missing docstring in public function + "D103", + # Missing docstring in public package + "D104", + # __magic__ methods are are often self-explanatory, allow missing docstrings + "D105", + # Missing docstring in public nested class + "D106", + # Missing docstring in __init__ + "D107", + ## Disable one in each pair of mutually incompatible rules + # We don’t want a blank line before a class docstring + "D203", + # 1 blank line required after class docstring + "D204", + # first line should end with a period [Bug: doesn't work with single-line docstrings] + # We want docstrings to start immediately after the opening triple quote + "D213", + # Section underline is over-indented ("{name}") + "D215", + # First line should end with a period + "D400", + # First line should be in imperative mood; try rephrasing + "D401", + # First word of the first line should be capitalized: {} -> {} + "D403", + # First word of the docstring should not be "This" + "D404", + # Section name should end with a newline ("{name}") + "D406", + # Missing dashed underline after section ("{name}") + "D407", + # Section underline should be in the line following the section's name ("{name}") + "D408", + # Section underline should match the length of its name ("{name}") + "D409", + # No blank lines allowed between a section header and its content ("{name}") + "D412", + # Missing blank line after last section ("{name}") + "D413", + # camcelcase imported as lowercase + "N813", + # module import not at top level of file + "E402", + # open()` should be replaced by `Path.open() + "PTH123", + # subprocess` call: check for execution of untrusted input - https://github.com/PyCQA/bandit/issues/333 + "S603", + # Starting a process with a partial executable path + "S607" +] + +[tool.ruff.lint.pydocstyle] +convention = "google" + +[tool.ruff.lint.per-file-ignores] +"docs/*" = ["I", "S101"] +"tests/**/*.py" = [ + "D", # docstrings are allowed to look a bit off + "S101", # asserts allowed in tests... + "ARG", # Unused function args -> fixtures nevertheless are functionally relevant... + "FBT", # Don't care about booleans as positional arguments in tests, e.g. via @pytest.mark.parametrize() + "PLR2004", # Magic value used in comparison, ... + "S311", # Standard pseudo-random generators are not suitable for cryptographic purposes +] +"*/__init__.py" = ["F401"] diff --git a/tests/test_package_name.py b/tests/test_package_name.py index 5bad2f0..aba1368 100644 --- a/tests/test_package_name.py +++ b/tests/test_package_name.py @@ -2,4 +2,4 @@ def test_get_package_name(): - get_package_name() == "laminci" + assert get_package_name() == "laminci"