Skip to content
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

DM-24780: add initial mypy support. #276

Merged
merged 6 commits into from
May 8, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ bin/*
.pytest_cache/
.mypy_cache/
.idea/
.vscode/
.vscode/
mypy.log
5 changes: 4 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@ matrix:
- python: '3.7'
install:
- pip install flake8
script: flake8
- pip install mypy
script:
- flake8
- cd python && mypy -p lsst.daf.butler
6 changes: 6 additions & 0 deletions python/SConscript
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# -*- python -*-
from lsst.sconsUtils import state
mypy = state.env.Command("../mypy.log", "lsst/daf/butler",
"cd python && mypy -p lsst.daf.butler 2>&1 | tee -a ../mypy.log")

state.env.Alias("mypy", mypy)
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/registry/interfaces/_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from .._collectionType import CollectionType

if TYPE_CHECKING:
from .database import Database, StaticTablesContext
from ._database import Database, StaticTablesContext


class MissingCollectionError(Exception):
Expand Down
20 changes: 11 additions & 9 deletions python/lsst/daf/butler/registry/interfaces/_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
Any,
Dict,
Iterable,
Iterator,
List,
Optional,
Sequence,
Expand All @@ -47,7 +48,7 @@
from .._exceptions import ConflictingDefinitionError


def _checkExistingTableDefinition(name: str, spec: ddl.TableSpec, inspection: Dict[str, Any]):
def _checkExistingTableDefinition(name: str, spec: ddl.TableSpec, inspection: List[Dict[str, Any]]):
"""Test that the definition of a table in a `ddl.TableSpec` and from
database introspection are consistent.

Expand Down Expand Up @@ -95,7 +96,7 @@ class StaticTablesContext:

def __init__(self, db: Database):
self._db = db
self._foreignKeys = []
self._foreignKeys: List[Tuple[sqlalchemy.schema.Table, sqlalchemy.schema.ForeignKeyConstraint]] = []
self._inspector = sqlalchemy.engine.reflection.Inspector(self._db._connection)
self._tableNames = frozenset(self._inspector.get_table_names(schema=self._db.namespace))

Expand Down Expand Up @@ -136,7 +137,8 @@ def addTableTuple(self, specs: Tuple[ddl.TableSpec, ...]) -> Tuple[sqlalchemy.sc
is just a factory for `type` objects, not an actual type itself,
we cannot represent this with type annotations.
"""
return specs._make(self.addTable(name, spec) for name, spec in zip(specs._fields, specs))
return specs._make(self.addTable(name, spec) # type: ignore
for name, spec in zip(specs._fields, specs)) # type: ignore
Comment on lines +141 to +142
Copy link
Member

Choose a reason for hiding this comment

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

I feel like there's probably something better than # type: ignore, but collections.namedtuple is very weird in the type system, and I couldn't come up with anything better either.



class Database(ABC):
Expand Down Expand Up @@ -173,8 +175,7 @@ class Database(ABC):

`Database` itself has several underscore-prefixed attributes:

- ``_cs``: SQLAlchemy objects representing the connection and transaction
state.
- ``_connection``: SQLAlchemy object representing the connection.
- ``_metadata``: the `sqlalchemy.schema.MetaData` object representing
the tables and other schema entities.

Expand All @@ -188,7 +189,7 @@ def __init__(self, *, origin: int, connection: sqlalchemy.engine.Connection,
self.origin = origin
self.namespace = namespace
self._connection = connection
self._metadata = None
self._metadata: Optional[sqlalchemy.schema.MetaData] = None

@classmethod
def makeDefaultUri(cls, root: str) -> Optional[str]:
Expand Down Expand Up @@ -299,7 +300,7 @@ def fromConnection(cls, connection: sqlalchemy.engine.Connection, *, origin: int
raise NotImplementedError()

@contextmanager
def transaction(self, *, interrupting: bool = False) -> None:
def transaction(self, *, interrupting: bool = False) -> Iterator:
"""Return a context manager that represents a transaction.

Parameters
Expand All @@ -326,7 +327,7 @@ def transaction(self, *, interrupting: bool = False) -> None:
raise

@contextmanager
def declareStaticTables(self, *, create: bool) -> StaticTablesContext:
def declareStaticTables(self, *, create: bool) -> Iterator[StaticTablesContext]:
"""Return a context manager in which the database's static DDL schema
can be declared.

Expand Down Expand Up @@ -702,7 +703,7 @@ def sync(self, table: sqlalchemy.schema.Table, *,
compared: Optional[Dict[str, Any]] = None,
extra: Optional[Dict[str, Any]] = None,
returning: Optional[Sequence[str]] = None,
) -> Tuple[Optional[Dict[str, Any], bool]]:
) -> Tuple[Optional[Dict[str, Any]], bool]:
"""Insert into a table as necessary to ensure database contains
values equivalent to the given ones.

Expand Down Expand Up @@ -919,6 +920,7 @@ def insert(self, table: sqlalchemy.schema.Table, *rows: dict, returnIds: bool =
raise ReadOnlyDatabaseError(f"Attempt to insert into read-only database '{self}'.")
if not returnIds:
self._connection.execute(table.insert(), *rows)
return None
else:
sql = table.insert()
return [self._connection.execute(sql, row).inserted_primary_key[0] for row in rows]
Expand Down
3 changes: 2 additions & 1 deletion python/lsst/daf/butler/registry/nameShrinker.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
__all__ = ["NameShrinker"]

import hashlib
from typing import Dict


class NameShrinker:
Expand All @@ -44,7 +45,7 @@ class NameShrinker:
def __init__(self, maxLength: int, hashSize: int = 4):
self.maxLength = maxLength
self.hashSize = hashSize
self._names = {}
self._names: Dict[str, str] = {}

def shrink(self, original: str) -> str:
"""Shrink a name and remember the mapping between the original name and
Expand Down
4 changes: 3 additions & 1 deletion python/lsst/daf/butler/registry/simpleQuery.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from typing import (
Any,
ClassVar,
List,
Optional,
Union,
Expand All @@ -42,7 +43,8 @@ class Select:
"""Tag class used to indicate that a field should be returned in
a SELECT query.
"""
pass

Or: ClassVar


Select.Or = Union[T, Type[Select]]
Expand Down
20 changes: 20 additions & 0 deletions python/mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[mypy]
warn_unused_configs = True
namespace_packages = True

[mypy-sqlalchemy.*]
ignore_missing_imports = True

[mypy-astropy.*]
ignore_missing_imports = True

[mypy-boto3]
ignore_missing_imports = True

[mypy-lsst.*]
ignore_missing_imports = True
ignore_errors = True

[mypy-lsst.daf.butler.registry.interfaces.*]
ignore_missing_imports = False
ignore_errors = False