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

Fix many, but not all, typing failures #168

Merged
merged 1 commit into from
Aug 1, 2024
Merged

Fix many, but not all, typing failures #168

merged 1 commit into from
Aug 1, 2024

Conversation

shrik450
Copy link
Collaborator

This is a first pass at making this project type-checkable, and fixes much of the "low-hanging fruit" type errors. A few bugs were also discovered during this process and have been fixed.

Notes:

  • I'm using pyright to typecheck instead of mypy, as pyright ships with the default VSCode Python language server, is faster than mypy and doesn't miss anything mypy does.
  • The remaining type issues require some more thinking, and I'll discuss the challenges more in Should pass mypy type-checking #36.

@shrik450 shrik450 requested review from kdumontnu and McRaeAlex July 25, 2024 15:21
@shrik450 shrik450 self-assigned this Jul 25, 2024
@shrik450 shrik450 requested a review from a team as a code owner July 25, 2024 15:21
@shrik450 shrik450 force-pushed the su/mypy-1 branch 2 times, most recently from b21299c to bd75b60 Compare July 25, 2024 15:29
Copy link

github-actions bot commented Jul 25, 2024

Coverage Summary

Total Project Coverage

  • Line Coverage: 85.70% (1768/2063)
  • Branch Coverage: 72.36% (453/626)

Coverage by File

File Line Coverage Branch Coverage Lines (Covered/Total) Branches (Covered/Total)
allspice/__init__.py 100.00% 100.00% 5/5 0/0
allspice/allspice.py 80.98% 67.57% 166/205 50/74
allspice/apiobject.py 83.71% 61.11% 1012/1209 165/270
allspice/baseapiobject.py 87.25% 84.62% 89/102 44/52
allspice/exceptions.py 100.00% 100.00% 14/14 0/0
allspice/ratelimiter.py 100.00% 100.00% 22/22 4/4
allspice/utils/__init__.py 100.00% 100.00% 0/0 0/0
allspice/utils/bom_generation.py 95.07% 84.21% 135/142 64/76
allspice/utils/core.py 94.12% 50.00% 16/17 1/2
allspice/utils/list_components.py 88.40% 83.85% 259/293 109/130
allspice/utils/netlist_generation.py 92.59% 88.89% 50/54 16/18

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • allspice/allspice.py (100%)
  • allspice/apiobject.py (60.0%): Missing lines 140,368,1516,1519,1711,1737,1849,2020,2023,2302-2303,2472,2475,2478
  • allspice/baseapiobject.py (90.3%): Missing lines 11,40,55
  • allspice/utils/bom_generation.py (100%)
  • allspice/utils/list_components.py (83.3%): Missing lines 221
  • allspice/utils/netlist_generation.py (83.3%): Missing lines 57

Summary

  • Total: 86 lines
  • Missing: 19 lines
  • Coverage: 77%
Diff Coverage Details

allspice/apiobject.py

Lines 136-144

  136             self.allspice_client.logger.info("Successfully created Repository %s " % result["name"])
  137         else:
  138             self.allspice_client.logger.error(result["message"])
  139             raise Exception("Repository not created... (gitea: %s)" % result["message"])
! 140         return Repository.parse_response(self.allspice_client, result)
  141 
  142     def get_repositories(self) -> List["Repository"]:
  143         results = self.allspice_client.requests_get_paginated(
  144             Organization.ORG_REPOS_REQUEST % self.username

Lines 364-372

  364             self.allspice_client.logger.info("Successfully created Repository %s " % result["name"])
  365         else:
  366             self.allspice_client.logger.error(result["message"])
  367             raise Exception("Repository not created... (gitea: %s)" % result["message"])
! 368         return Repository.parse_response(self.allspice_client, result)
  369 
  370     def get_repositories(self) -> List["Repository"]:
  371         """Get all Repositories owned by this User."""
  372         url = f"/users/{self.username}/repos"

Lines 1512-1523

  1512 
  1513     def __eq__(self, other):
  1514         if not isinstance(other, Comment):
  1515             return False
! 1516         return self.repository == other.repository and self.id == other.id
  1517 
  1518     def __hash__(self):
! 1519         return hash(self.repository) ^ hash(self.id)
  1520 
  1521     @classmethod
  1522     def request(cls, allspice_client, owner: str, repo: str, id: str) -> "Comment":
  1523         return cls._request(allspice_client, {"owner": owner, "repo": repo, "id": id})

Lines 1707-1715

  1707     @cached_property
  1708     def _fields_for_path(self) -> dict[str, str]:
  1709         matches = self.URL_REGEXP.search(self.url)
  1710         if not matches:
! 1711             raise ValueError(f"Invalid commit URL: {self.url}")
  1712 
  1713         return {
  1714             "owner": matches.group(1),
  1715             "repo": matches.group(2),

Lines 1733-1741

  1733 
  1734         try:
  1735             return cls(value)
  1736         except ValueError:
! 1737             return value
  1738 
  1739 
  1740 class CommitStatus(ReadonlyApiObject):
  1741     context: str

Lines 1845-1853

  1845 
  1846     def __eq__(self, other):
  1847         if not isinstance(other, Issue):
  1848             return False
! 1849         return self.repository == other.repository and self.id == other.id
  1850 
  1851     def __hash__(self):
  1852         return hash(self.repository) ^ hash(self.id)

Lines 2016-2027

  2016 
  2017     def __eq__(self, other):
  2018         if not isinstance(other, DesignReview):
  2019             return False
! 2020         return self.repository == other.repository and self.id == other.id
  2021 
  2022     def __hash__(self):
! 2023         return hash(self.repository) ^ hash(self.id)
  2024 
  2025     @classmethod
  2026     def parse_response(cls, allspice_client, result) -> "DesignReview":
  2027         api_object = super().parse_response(allspice_client, result)

Lines 2298-2307

  2298         id: Optional[int] = None,
  2299     ) -> Release:
  2300         args = {"owner": owner, "repo": repo, "id": id}
  2301         release_response = cls._get_gitea_api_object(allspice_client, args)
! 2302         repository = Repository.request(allspice_client, owner, repo)
! 2303         release = cls.parse_response(allspice_client, release_response, repository)
  2304         return release
  2305 
  2306     def commit(self):
  2307         args = {"owner": self.repo.owner.name, "repo": self.repo.name, "id": self.id}

Lines 2468-2482

  2468     def __init__(self, allspice_client):
  2469         super().__init__(allspice_client)
  2470 
  2471     def __eq__(self, other):
! 2472         if not isinstance(other, Content):
  2473             return False
  2474 
! 2475         return self.sha == other.sha and self.name == other.name
  2476 
  2477     def __hash__(self):
! 2478         return hash(self.sha) ^ hash(self.name)
  2479 
  2480 
  2481 Ref = Union[Branch, Commit, str]

allspice/baseapiobject.py

Lines 7-15

   7 except ImportError:
   8     from typing import Self
   9 
  10 if TYPE_CHECKING:
! 11     from allspice.allspice import AllSpice
  12 
  13 from .exceptions import MissingEqualityImplementation, ObjectIsInvalid, RawRequestEndpointMissing
  14 

Lines 36-44

  36     @classmethod
  37     def request(cls, allspice_client: AllSpice) -> Self:
  38         # This never would've worked, so maybe we should remove this function
  39         # outright.
! 40         return cls._request(allspice_client)
  41 
  42     @classmethod
  43     def _request(cls, allspice_client: AllSpice, args: Mapping) -> Self:
  44         result = cls._get_gitea_api_object(allspice_client, args)

Lines 51-59

  51         if hasattr(cls, "API_OBJECT"):
  52             raw_request_endpoint = getattr(cls, "API_OBJECT")
  53             return allspice_client.requests_get(raw_request_endpoint.format(**args))
  54         else:
! 55             raise RawRequestEndpointMissing()
  56 
  57     @classmethod
  58     def parse_response(cls, allspice_client: AllSpice, result: Mapping) -> Self:
  59         # allspice_client.logger.debug("Found api object of type %s (id: %s)" % (type(cls), id))

allspice/utils/list_components.py

Lines 217-225

  217 
  218     if variant is not None:
  219         if variant_details is None:
  220             # This should never happen, but mypy doesn't know that.
! 221             raise ValueError(f"Variant {variant} not found in PrjPcb file.")
  222 
  223         components = _apply_variations(components, variant_details, allspice_client.logger)
  224 
  225     return components

allspice/utils/netlist_generation.py

Lines 53-61

  53     allspice_client.logger.info(f"Generating netlist for {repository.name=} on {ref=}")
  54     allspice_client.logger.info(f"Fetching {pcb_file=}")
  55 
  56     if isinstance(pcb_file, Content):
! 57         pcb_file_path = pcb_file.path
  58     else:
  59         pcb_file_path = pcb_file
  60 
  61     pcb_components = _extract_all_pcb_components(

@McRaeAlex McRaeAlex added the house keeping Internal code improvement, refactoring, or other house keeping label Jul 30, 2024
@shrik450 shrik450 merged commit faff47d into main Aug 1, 2024
4 checks passed
@shrik450 shrik450 deleted the su/mypy-1 branch August 1, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
house keeping Internal code improvement, refactoring, or other house keeping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants