Skip to content
This repository has been archived by the owner on Dec 15, 2023. It is now read-only.

chore: reorder pre-commit & pyupgrade --py39 #567

Merged
merged 3 commits into from
Mar 14, 2023
Merged

Conversation

MindTooth
Copy link
Collaborator

@MindTooth MindTooth commented Mar 11, 2023

I have a complaint from MyPy:

src/core/repository/repository.py:83: error: Function "core.repository.repository.AbstractProjectRepository.list" is not valid as a type  [valid-type]
src/core/repository/repository.py:83: note: Perhaps you need "Callable[...]" or a callback protocol?
Found 1 error in 1 file (checked 1 source file)
❯ nina --dev
Traceback (most recent call last):
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/bin/nina", line 5, in <module>
    from nina.run import main
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/nina/run.py", line 9, in <module>
    from core.main import main as core_main  # type: ignore
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/main.py", line 14, in <module>
    import core.api
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/api/__init__.py", line 3, in <module>
    from .api import *
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/api/api.py", line 29, in <module>
    from core import model, services
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/services.py", line 24, in <module>
    from core.repository import SqlAlchemyProjectRepository as ProjectRepository
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/repository/__init__.py", line 3, in <module>
    from .repository import *
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/repository/repository.py", line 23, in <module>
    class AbstractProjectRepository(abc.ABC):
  File "/Users/birgerjn/Developer/fish-code/.direnv/python-3.9.6/lib/python3.9/site-packages/core/repository/repository.py", line 83, in AbstractProjectRepository
    def _list(self) -> list[model.Project]:  # pragma: no cover
TypeError: 'function' object is not subscriptable

Not sure what this tells me.

I see that it's not implemented. Warranted a removal?

@MindTooth MindTooth added help wanted Extra attention is needed code::quality doing python Pull requests that update Python code labels Mar 11, 2023
@MindTooth MindTooth requested a review from tomrtk March 11, 2023 10:06
@tomrtk
Copy link
Owner

tomrtk commented Mar 13, 2023

To solve the typeerror at runtime we need from __future__ import annotations in the file or python tries to index the function list.

I have a complaint from MyPy:
...
Not sure what this tells me.

I see that it's not implemented. Warranted a removal?

The thought of the abstract class was to define the api for an implementation of the repository.
to solve the typing issue we can define a Protocol of the minimal signature a repository needs.

something like this seems to work, I can make a PR if you want.

diff --git a/src/core/repository/repository.py b/src/core/repository/repository.py
index d63b66c..17ff387 100644
--- a/src/core/repository/repository.py
+++ b/src/core/repository/repository.py
@@ -5,7 +5,7 @@ abstract class defines the methods all repositories needs to implement.
 """
 import abc
 import logging
-from typing import List, Optional
+from typing import List, Optional, Protocol
 
 from sqlalchemy.orm.session import Session
 
@@ -20,7 +20,21 @@ class NotFound(Exception):
     pass
 
 
-class AbstractProjectRepository(abc.ABC):
+class _ProjectRepositroy(Protocol):
+    def _add(self, project: model.Project) -> model.Project:
+        ...
+
+    def _save(self) -> None:
+        ...
+
+    def _get(self, reference: int) -> Optional[model.Project]:
+        ...
+
+    def _list(self) -> list[model.Project]:
+        ...
+
+
+class AbstractProjectRepository(abc.ABC, _ProjectRepositroy):
     """Abstract project repository defining minimum API of repository."""
 
     def __init__(self) -> None:
@@ -65,24 +79,6 @@ class AbstractProjectRepository(abc.ABC):
         """Get a list off all Projects in repository."""
         return self._list()
 
-    @abc.abstractmethod
-    def _add(self, project: model.Project) -> model.Project:  # pragma: no cover
-        raise NotImplementedError
-
-    @abc.abstractmethod
-    def _save(self) -> None:  # pragma: no cover
-        raise NotImplementedError
-
-    @abc.abstractmethod
-    def _get(
-        self, reference: int
-    ) -> Optional[model.Project]:  # pragma: no cover
-        raise NotImplementedError
-
-    @abc.abstractmethod
-    def _list(self) -> list[model.Project]:  # pragma: no cover
-        raise NotImplementedError
-
     def __len__(self) -> int:
         """Get number of projects in repository.

Also if the logging and __len__ is move to SqlAlchemyProjectRepository the abstract class can be removed and the protocol defines the api.

@MindTooth
Copy link
Collaborator Author

Feel free to open a pull request. 😁 Thanks for the explanation.

- replace abstract class with Protocol

- rename repository.py to project.py
@MindTooth
Copy link
Collaborator Author

Did a rebase and force-push to remove my failed attempt at solving the issue. Seems to retain the author. Nice work!

src/core/model.py Show resolved Hide resolved
@MindTooth MindTooth merged commit 19c688b into master Mar 14, 2023
@MindTooth MindTooth deleted the chore/pre_commit branch March 14, 2023 21:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
code::quality doing help wanted Extra attention is needed python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants