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

(WIP) Add in Type-Safety #135

Closed
wants to merge 13 commits into from
Closed

(WIP) Add in Type-Safety #135

wants to merge 13 commits into from

Conversation

max-muoto
Copy link
Contributor

No description provided.

@@ -1,10 +1,7 @@
import copy
import os
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ruff got ran here.

@@ -62,6 +58,8 @@ python = ">=3.8.0,<4"
django = ">=3"

[tool.poetry.dev-dependencies]
pyright = "^1.1.335"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can pin these if you like.

@@ -276,10 +307,10 @@ def schema(*schemas: str, databases: Union[List[str], None] = None):
yield


schema.session = _schema_session
schema.session = _schema_session # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix here is making these class-based context managers, can do so in a follow-up PR. Just didn't want to refactor them here.

from pgtrigger import utils

_unset = object()

Unset = NewType("Unset", object)
Copy link
Contributor Author

@max-muoto max-muoto Nov 12, 2023

Choose a reason for hiding this comment

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

NewType helps prevents bugs by accepting object. Accepting object as a type-hint significantly reduces the type-safety of a method, so this enforces that we explicitly are setting it our unset object. Before having this I had actually incorrectly typed some things.

@max-muoto max-muoto closed this Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant