Skip to content
This repository has been archived by the owner on Jul 13, 2022. It is now read-only.

Better static analysis support #19

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Better static analysis support #19

wants to merge 11 commits into from

Conversation

bgimby-ns
Copy link

@bgimby-ns bgimby-ns commented Mar 14, 2022

Overview of changes

  • Adds a mypy pre-commit job
  • Adds and changes type hints where they caused errors
  • Made functional changes in a couple of spots where the errors looked like they could cause real exceptions at runtime
  • Adds an additional way to construct database models (by subclassing DatabaseModel) which static analyzers can understand

@bgimby-ns bgimby-ns changed the title [WIP] Add mypy support Add mypy support Mar 14, 2022
@@ -39,18 +54,18 @@
from pynocular.nested_database_model import NestedDatabaseModel


def is_valid_uuid(string: str) -> bool:
"""Check if a string is a valid UUID
def is_valid_uuid(obj: Any) -> TypeGuard["UUID_STR"]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does TypeGuard do? The return here is still bool


The false positives are due to a limitation of python's type hint system that
doesn't allow for dynamic class modification. The current workaround is to use
the `BaseModel_database_model_hint` class provided by Pynocular, which is
Copy link
Collaborator

Choose a reason for hiding this comment

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

That class name doesn't follow Python convention and is also very wordy. We shouldn't have to do this in order get mypy to work. If the issue is the dynamic class modification we should look into fixing that first before patching over it

return str(v)
else:
raise ValueError("invalid UUID string")


SelfType = TypeVar("SelfType", bound="DatabaseModel")
Copy link
Collaborator

Choose a reason for hiding this comment

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

whats the purpose of this? A comment would be helpful

@bgimby-ns bgimby-ns changed the title Add mypy support Better static analysis support Mar 21, 2022
other: int = 0

class NestedModel(DatabaseModel, table_name = "table2", database_info=DBInfo("type")):
if TYPE_CHECKING:
Copy link
Author

@bgimby-ns bgimby-ns Mar 21, 2022

Choose a reason for hiding this comment

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

This is a way to get the nested models to have type checking/IDE support, but it's not needed if you don't care about that

@jdrake jdrake mentioned this pull request Apr 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants