diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index efe4103..f131fbd 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -41,8 +41,7 @@ jobs: shell: bash -l {0} run: | conda install -y -q \ - flake8 \ - pytest pytest-flake8 pytest-xdist pytest-openfiles pytest-cov + pytest pytest-xdist pytest-openfiles pytest-cov - name: List installed packages shell: bash -l {0} diff --git a/.github/workflows/build_docs.yaml b/.github/workflows/build_docs.yaml new file mode 100644 index 0000000..d6c0577 --- /dev/null +++ b/.github/workflows/build_docs.yaml @@ -0,0 +1,48 @@ +name: docs + +on: + push: + branches: + - main + pull_request: + +jobs: + build_sphinx_docs: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + # Need to clone everything for the git tags. + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v3 + with: + python-version: '3.10' + cache: "pip" + cache-dependency-path: "setup.cfg" + + - name: Install sqlite + run: sudo apt-get install sqlite libyaml-dev + + - name: Update pip/wheel infrastructure + run: | + python -m pip install --upgrade pip + pip install wheel + + - name: Install dependencies + run: | + pip install -r requirements.txt + + - name: Build and install + run: pip install --no-deps -v . + + - name: Install documenteer + run: pip install 'documenteer[pipelines]>0.8,<0.9' + + - name: Build documentation + env: + DAF_BUTLER_MIGRATE_DIR: . + DAF_BUTLER_MIGRATE_MIGRATIONS: ./migrations + working-directory: ./doc + run: package-docs build -n -W diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 796ef92..6c463ee 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -9,3 +9,8 @@ on: jobs: call-workflow: uses: lsst/rubin_workflows/.github/workflows/lint.yaml@main + ruff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: chartboost/ruff-action@v1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ee17455..dbfb184 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,7 +6,7 @@ repos: - id: end-of-file-fixer - id: trailing-whitespace - repo: https://github.com/psf/black - rev: 23.1.0 + rev: 23.3.0 hooks: - id: black # It is recommended to specify the latest version of Python @@ -19,7 +19,8 @@ repos: hooks: - id: isort name: isort (python) - - repo: https://github.com/PyCQA/flake8 - rev: 6.0.0 + - repo: https://github.com/astral-sh/ruff-pre-commit + # Ruff version. + rev: v0.0.277 hooks: - - id: flake8 + - id: ruff diff --git a/pyproject.toml b/pyproject.toml index 29ac6eb..205016e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,3 +63,46 @@ line_length = 110 [tool.lsst_versions] write_to = "python/lsst/daf/butler_migrate/version.py" + +[tool.ruff] +exclude = [ + "__init__.py", + "migrations/*/*.py", +] +ignore = [ + "N802", + "N803", + "N806", + "N812", + "N815", + "N816", + "N999", + "D107", + "D105", + "D102", + "D104", + "D100", + "D200", + "D205", + "D400", +] +line-length = 110 +select = [ + "E", # pycodestyle + "F", # pycodestyle + "N", # pep8-naming + "W", # pycodestyle + "D", # pydocstyle + "UP", + "C4", +] +target-version = "py310" +extend-select = [ + "RUF100", # Warn about unused noqa +] + +[tool.ruff.pycodestyle] +max-doc-length = 79 + +[tool.ruff.pydocstyle] +convention = "numpy" diff --git a/python/lsst/daf/butler_migrate/butler_attributes.py b/python/lsst/daf/butler_migrate/butler_attributes.py index 0f500db..e0629e1 100644 --- a/python/lsst/daf/butler_migrate/butler_attributes.py +++ b/python/lsst/daf/butler_migrate/butler_attributes.py @@ -111,7 +111,7 @@ def update(self, name: str, value: str) -> int: return self._connection.execute(sql).rowcount def update_manager_version(self, manager: str, version: str) -> None: - """Convenience method for updating version for the specified manager. + """Update version for the specified manager. Parameters ---------- @@ -163,7 +163,7 @@ def get_dimensions_json(self) -> dict[str, Any]: return config def update_dimensions_json(self, update_config: Callable[[dict], dict]) -> None: - """Updates dimensions definitions in dimensions.json. + """Update dimensions definitions in dimensions.json. Parameters ---------- diff --git a/python/lsst/daf/butler_migrate/config.py b/python/lsst/daf/butler_migrate/config.py index fc98136..2b5e5a4 100644 --- a/python/lsst/daf/butler_migrate/config.py +++ b/python/lsst/daf/butler_migrate/config.py @@ -23,7 +23,7 @@ import logging import os -from typing import Any, Dict, Optional +from typing import Any from alembic.config import Config @@ -42,11 +42,11 @@ def from_mig_path( cls, mig_path: str, *args: Any, - repository: Optional[str] = None, - db: Optional[database.Database] = None, - single_tree: Optional[str] = None, - one_shot_tree: Optional[str] = None, - migration_options: Optional[Dict[str, str]] = None, + repository: str | None = None, + db: database.Database | None = None, + single_tree: str | None = None, + one_shot_tree: str | None = None, + migration_options: dict[str, str] | None = None, **kwargs: Any, ) -> MigAlembicConfig: """Create new configuration object. diff --git a/python/lsst/daf/butler_migrate/migrate.py b/python/lsst/daf/butler_migrate/migrate.py index 5c68720..ba6af8c 100644 --- a/python/lsst/daf/butler_migrate/migrate.py +++ b/python/lsst/daf/butler_migrate/migrate.py @@ -22,7 +22,6 @@ from __future__ import annotations import os -from typing import Dict, List, Optional class MigrationTrees: @@ -44,7 +43,7 @@ class MigrationTrees: """Name of envvar for location of a package containing default migrations. """ - def __init__(self, mig_path: Optional[str] = None): + def __init__(self, mig_path: str | None = None): if mig_path is None: self.mig_path = self.migrations_folder() else: @@ -139,7 +138,7 @@ def one_shot_version_location(self, one_shot_tree: str, *, relative: bool = True path = os.path.join(self.mig_path, path) return path - def regular_version_locations(self, *, relative: bool = True) -> Dict[str, str]: + def regular_version_locations(self, *, relative: bool = True) -> dict[str, str]: """Return locations for regular migrations. Parameters @@ -154,7 +153,7 @@ def regular_version_locations(self, *, relative: bool = True) -> Dict[str, str]: Dictionary where key is a manager name (e.g. "datasets") and value is the location of a folder with migration scripts. """ - locations: Dict[str, str] = {} + locations: dict[str, str] = {} for entry in os.scandir(self.mig_path): if entry.is_dir() and entry.name not in ("_alembic", "_oneshot"): path = entry.name @@ -163,7 +162,7 @@ def regular_version_locations(self, *, relative: bool = True) -> Dict[str, str]: locations[entry.name] = path return locations - def one_shot_locations(self, manager: Optional[str] = None, *, relative: bool = True) -> Dict[str, str]: + def one_shot_locations(self, manager: str | None = None, *, relative: bool = True) -> dict[str, str]: """Return locations for one-shot migrations for specific manager. Parameters @@ -180,7 +179,7 @@ def one_shot_locations(self, manager: Optional[str] = None, *, relative: bool = Dictionary where key is a one-shot tree name and value is the location of a folder with migration scripts. """ - locations: Dict[str, str] = {} + locations: dict[str, str] = {} one_shot_loc = os.path.join(self.mig_path, "_oneshot") if not os.access(one_shot_loc, os.F_OK): @@ -205,7 +204,7 @@ def one_shot_locations(self, manager: Optional[str] = None, *, relative: bool = locations[manager + "/" + entry.name] = path return locations - def version_locations(self, one_shot_tree: Optional[str] = None, *, relative: bool = True) -> List[str]: + def version_locations(self, one_shot_tree: str | None = None, *, relative: bool = True) -> list[str]: """Return list of folders for version_locations. Parameters diff --git a/python/lsst/daf/butler_migrate/script/migrate_add_tree.py b/python/lsst/daf/butler_migrate/script/migrate_add_tree.py index 4f4fc6a..1deba2e 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_add_tree.py +++ b/python/lsst/daf/butler_migrate/script/migrate_add_tree.py @@ -64,7 +64,6 @@ def migrate_add_tree(tree_name: str, mig_path: str, one_shot: bool) -> None: all trees the root of the tree is labelled with the manager name (e.g. "datasets"). """ - trees = migrate.MigrationTrees(mig_path) manager = tree_name diff --git a/python/lsst/daf/butler_migrate/script/migrate_current.py b/python/lsst/daf/butler_migrate/script/migrate_current.py index 3d59f16..6f525e0 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_current.py +++ b/python/lsst/daf/butler_migrate/script/migrate_current.py @@ -25,7 +25,6 @@ from __future__ import annotations import logging -from typing import Optional from alembic import command @@ -34,7 +33,7 @@ _LOG = logging.getLogger(__name__) -def migrate_current(repo: str, mig_path: str, verbose: bool, butler: bool, namespace: Optional[str]) -> None: +def migrate_current(repo: str, mig_path: str, verbose: bool, butler: bool, namespace: str | None) -> None: """Display current revisions for a database. Parameters diff --git a/python/lsst/daf/butler_migrate/script/migrate_downgrade.py b/python/lsst/daf/butler_migrate/script/migrate_downgrade.py index 5cdfeac..4195c38 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_downgrade.py +++ b/python/lsst/daf/butler_migrate/script/migrate_downgrade.py @@ -25,7 +25,6 @@ from __future__ import annotations import logging -from typing import Optional from alembic import command @@ -35,7 +34,7 @@ def migrate_downgrade( - repo: str, revision: str, mig_path: str, one_shot_tree: str, sql: bool, namespace: Optional[str] + repo: str, revision: str, mig_path: str, one_shot_tree: str, sql: bool, namespace: str | None ) -> None: """Downgrade schema to a specified revision. @@ -71,7 +70,7 @@ def migrate_downgrade( "Alembic version table does not exist, you may need to run `butler migrate stamp` first." ) - one_shot_arg: Optional[str] = None + one_shot_arg: str | None = None if one_shot_tree: one_shot_arg = one_shot_tree cfg = config.MigAlembicConfig.from_mig_path(mig_path, repository=repo, db=db, one_shot_tree=one_shot_arg) diff --git a/python/lsst/daf/butler_migrate/script/migrate_dump_schema.py b/python/lsst/daf/butler_migrate/script/migrate_dump_schema.py index 01c5669..8980e0d 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_dump_schema.py +++ b/python/lsst/daf/butler_migrate/script/migrate_dump_schema.py @@ -22,14 +22,13 @@ from __future__ import annotations import logging -from typing import List from .. import database _LOG = logging.getLogger(__name__) -def migrate_dump_schema(repo: str, table: List[str]) -> None: +def migrate_dump_schema(repo: str, table: list[str]) -> None: """Dump the schema of the registry database. Parameters diff --git a/python/lsst/daf/butler_migrate/script/migrate_revision.py b/python/lsst/daf/butler_migrate/script/migrate_revision.py index 93ac5a3..5b8bd92 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_revision.py +++ b/python/lsst/daf/butler_migrate/script/migrate_revision.py @@ -25,7 +25,6 @@ from __future__ import annotations import logging -from typing import Optional from alembic import command, util from alembic.script import ScriptDirectory @@ -89,7 +88,7 @@ def migrate_revision(mig_path: str, tree_name: str, manager_class: str, version: # New revision should be either at the head of manager branch or at the # root of the tree (to make a new manager branch). manager_branch = f"{tree_name}-{manager_class}" - branch_label: Optional[str] = None + branch_label: str | None = None splice = False if _revision_exists(scripts, manager_branch): head = f"{manager_branch}@head" diff --git a/python/lsst/daf/butler_migrate/script/migrate_set_namespace.py b/python/lsst/daf/butler_migrate/script/migrate_set_namespace.py index edfc55a..91c2dbc 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_set_namespace.py +++ b/python/lsst/daf/butler_migrate/script/migrate_set_namespace.py @@ -25,14 +25,13 @@ from __future__ import annotations import logging -from typing import Dict, Optional from .. import butler_attributes, database _LOG = logging.getLogger(__name__) -def migrate_set_namespace(repo: str, namespace: Optional[str], update: bool) -> None: +def migrate_set_namespace(repo: str, namespace: str | None, update: bool) -> None: """Display current revisions for a database. Parameters @@ -62,7 +61,7 @@ def migrate_set_namespace(repo: str, namespace: Optional[str], update: bool) -> f"Namespace is already defined ({db_namespace}), use --update option to replace it." ) - def update_namespace(config: Dict) -> Dict: + def update_namespace(config: dict) -> dict: """Update namespace attribute""" config["namespace"] = namespace return config diff --git a/python/lsst/daf/butler_migrate/script/migrate_stamp.py b/python/lsst/daf/butler_migrate/script/migrate_stamp.py index babffd3..fe076a5 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_stamp.py +++ b/python/lsst/daf/butler_migrate/script/migrate_stamp.py @@ -25,7 +25,6 @@ from __future__ import annotations import logging -from typing import Dict, Optional from alembic import command @@ -35,7 +34,7 @@ def migrate_stamp( - repo: str, mig_path: str, purge: bool, dry_run: bool, namespace: Optional[str], manager: Optional[str] + repo: str, mig_path: str, purge: bool, dry_run: bool, namespace: str | None, manager: str | None ) -> None: """Stamp alembic revision table with current registry versions. @@ -66,7 +65,7 @@ def migrate_stamp( manager_versions = db.manager_versions(namespace) - revisions: Dict[str, str] = {} + revisions: dict[str, str] = {} for mgr_name, (klass, version, rev_id) in manager_versions.items(): _LOG.debug("found revision (%s, %s, %s) -> %s", mgr_name, klass, version, rev_id) revisions[mgr_name] = rev_id diff --git a/python/lsst/daf/butler_migrate/script/migrate_trees.py b/python/lsst/daf/butler_migrate/script/migrate_trees.py index 8003da2..efbe6fc 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_trees.py +++ b/python/lsst/daf/butler_migrate/script/migrate_trees.py @@ -24,8 +24,6 @@ from __future__ import annotations -from typing import Dict - from alembic.script import Script, ScriptDirectory from .. import config, migrate @@ -51,7 +49,7 @@ def migrate_trees(mig_path: str, verbose: bool, one_shot: bool) -> None: scripts = ScriptDirectory.from_config(cfg) bases = scripts.get_bases() - bases_map: Dict[str, Script] = {} + bases_map: dict[str, Script] = {} for name in bases: revision = scripts.get_revision(name) assert revision is not None, "Script for a known base must exist" diff --git a/python/lsst/daf/butler_migrate/script/migrate_upgrade.py b/python/lsst/daf/butler_migrate/script/migrate_upgrade.py index 87cba7c..f9f198a 100644 --- a/python/lsst/daf/butler_migrate/script/migrate_upgrade.py +++ b/python/lsst/daf/butler_migrate/script/migrate_upgrade.py @@ -25,7 +25,6 @@ from __future__ import annotations import logging -from typing import Dict, Optional from alembic import command @@ -40,8 +39,8 @@ def migrate_upgrade( mig_path: str, one_shot_tree: str, sql: bool, - namespace: Optional[str], - options: Optional[Dict[str, str]], + namespace: str | None, + options: dict[str, str] | None, ) -> None: """Upgrade schema to a specified revision. @@ -77,7 +76,7 @@ def migrate_upgrade( "Alembic version table does not exist, you may need to run `butler migrate stamp` first." ) - one_shot_arg: Optional[str] = None + one_shot_arg: str | None = None if one_shot_tree: one_shot_arg = one_shot_tree cfg = config.MigAlembicConfig.from_mig_path( diff --git a/python/lsst/daf/butler_migrate/script/rewrite_sqlite_registry.py b/python/lsst/daf/butler_migrate/script/rewrite_sqlite_registry.py index d32caaa..64cbb16 100644 --- a/python/lsst/daf/butler_migrate/script/rewrite_sqlite_registry.py +++ b/python/lsst/daf/butler_migrate/script/rewrite_sqlite_registry.py @@ -26,7 +26,6 @@ import os import tempfile from collections import defaultdict -from typing import Dict from lsst.daf.butler import Butler, Config, DatasetAssociation, DatasetId, DatasetRef, SkyPixDimension from lsst.daf.butler.datastores.fileDatastore import FileDatastore @@ -55,7 +54,6 @@ def rewrite_sqlite_registry(source: str) -> None: source : `str` URI to a SQLite butler repository. """ - # Create the source butler early so we can ask it questions # without assuming things. source_butler = Butler(source, writeable=False) @@ -88,22 +86,24 @@ def rewrite_sqlite_registry(source: str) -> None: # Force the new temporary butler repo to have an explicit path # to the existing datastore. This will only work for a FileDatastore # so try that and if it doesn't work warn but continue since either - # this is a new typo of datastore or a chained datastore of some kind. + # this is a new type of datastore or a chained datastore of some kind. # For now only handle the simple case. # NOTE: Execution Butler creation has a similar problem with working # out how to refer back to the original datastore. - if isinstance(source_butler.datastore, FileDatastore): - config["datastore", "root"] = str(source_butler.datastore.root) + if isinstance(source_butler._datastore, FileDatastore): + roots = source_butler.get_datastore_roots() + datastore_name, datastore_root = roots.popitem() + config["datastore", "root"] = str(datastore_root) # Force the name of the datastore since that should not # change from the source (and will if set from root) - config["datastore", "name"] = source_butler.datastore.name + config["datastore", "name"] = datastore_name else: log.warning( "Migration is designed for FileDatastore but encountered %s." " Attempting migration anyhow. It should work so long as is not used" " in config.", - str(type(source_butler.datastore)), + str(type(source_butler._datastore)), ) # Create a temp directory for the temporary butler (put it inside @@ -167,9 +167,9 @@ def transfer_everything(source_butler: Butler, dest_butler: Butler) -> None: Parameters ---------- - source_butler : `Butler` + source_butler : `~lsst.daf.butler.Butler` Butler to use as a source of information. - dest_butler : `Butler` + dest_butler : `~lsst.daf.butler.Butler` Butler to receive all the content. """ # Read all the datasets we are going to transfer, removing duplicates. @@ -193,7 +193,7 @@ def transfer_everything(source_butler: Butler, dest_butler: Butler) -> None: def create_associations( - source_butler: Butler, dest_butler: Butler, source_to_dest: Dict[DatasetId, DatasetRef] + source_butler: Butler, dest_butler: Butler, source_to_dest: dict[DatasetId, DatasetRef] ) -> None: """Create TAGGED and CALIBRATION collections in destination.""" # For every dataset type in destination, get TAGGED and CALIBRATION @@ -242,9 +242,9 @@ def transfer_non_datasets(source_butler: Butler, dest_butler: Butler) -> None: Parameters ---------- - source_butler : `Butler` + source_butler : `~lsst.daf.butler.Butler` Butler to extract information from. - dest_butler : `Butler` + dest_butler : `~lsst.daf.butler.Butler` Destination butler. """ # Use a string buffer to save on file I/O. This might result in twice diff --git a/tests/test_database.py b/tests/test_database.py index e5a945a..b92636e 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -101,7 +101,6 @@ class DatabaseTestCase(unittest.TestCase): def test_manager_versions(self) -> None: """Test for manager_versions() method""" - with make_revision_tables() as db_url: db = database.Database(db_url) manager_versions = db.manager_versions() @@ -119,7 +118,6 @@ def test_manager_versions(self) -> None: def test_alembic_revisions(self) -> None: """Test for alembic_revisions() method""" - with make_revision_tables() as db_url: db = database.Database(db_url) alembic_revisions = db.alembic_revisions() @@ -133,7 +131,6 @@ def test_alembic_revisions(self) -> None: def test_validate_revisions(self) -> None: """Test for validate_revisions() method""" - with make_revision_tables() as db_url: db = database.Database(db_url) db.validate_revisions() diff --git a/tests/test_digests.py b/tests/test_digests.py index 4af03fc..023ceab 100644 --- a/tests/test_digests.py +++ b/tests/test_digests.py @@ -30,7 +30,6 @@ class DigestsTestCase(unittest.TestCase): def test_tableSchemaRepr(self) -> None: """Test for _tableSchemaRepr method""" - engine = sqlalchemy.create_engine("sqlite://") mdata = sqlalchemy.schema.MetaData() @@ -58,7 +57,6 @@ def test_tableSchemaRepr_nullpk(self) -> None: """Test for _tableSchemaRepr method in case when PK is declared nullable. """ - engine = sqlalchemy.create_engine("sqlite://") # sqlalchemy allows nullable PK columns in table definition diff --git a/tests/test_dimensions_json.py b/tests/test_dimensions_json.py index ed407fb..4171e56 100644 --- a/tests/test_dimensions_json.py +++ b/tests/test_dimensions_json.py @@ -65,7 +65,7 @@ def make_butler_v0(self) -> Config: def load_data(self, registry: Registry, filename: str) -> None: """Load registry test data from filename in data folder.""" - with open(os.path.join(TESTDIR, "data", filename), "r") as stream: + with open(os.path.join(TESTDIR, "data", filename)) as stream: backend = YamlRepoImportBackend(stream, registry) backend.register() backend.load(datastore=None) diff --git a/tests/test_migrate.py b/tests/test_migrate.py index 08a66a2..2991251 100644 --- a/tests/test_migrate.py +++ b/tests/test_migrate.py @@ -63,7 +63,6 @@ class MigrateTestCase(unittest.TestCase): def test_MigrationTrees(self) -> None: """Test for MigrationTrees methods""" - with make_migrations() as mig_path: mtrees = migrate.MigrationTrees(mig_path)