Merge pull request #8464 from khoaguin/fix-mypy-issues-service-dataset

[Refactor] Fixing mypy issues of `service/dataset`

.pre-commit-config.yaml

@@ -172,7 +172,7 @@ repos:
       - id: mypy
         name: "mypy: syft"
         always_run: true
-        files: "^packages/syft/src/syft/serde|^packages/syft/src/syft/util/env.py|^packages/syft/src/syft/util/logger.py|^packages/syft/src/syft/util/markdown.py|^packages/syft/src/syft/util/notebook_ui/notebook_addons.py|^packages/syft/src/syft/service/warnings.py"
+        files: "^packages/syft/src/syft/serde|^packages/syft/src/syft/util/env.py|^packages/syft/src/syft/util/logger.py|^packages/syft/src/syft/util/markdown.py|^packages/syft/src/syft/util/notebook_ui/notebook_addons.py|^packages/syft/src/syft/service/warnings.py|^packages/syft/src/syft/service/dataset"
         #files: "^packages/syft/src/syft/serde"
         args: [
             "--follow-imports=skip",

packages/syft/src/syft/serde/third_party.py

@@ -96,8 +96,8 @@ def deserialize_dataframe(buf: bytes) -> DataFrame:
 
 
 def deserialize_series(blob: bytes) -> Series:
-    df = DataFrame.from_dict(deserialize(blob, from_bytes=True))
-    return df[df.columns[0]]
+    df: DataFrame = DataFrame.from_dict(deserialize(blob, from_bytes=True))
+    return Series(df[df.columns[0]])
 
 
 recursive_serde_register(

packages/syft/src/syft/service/dataset/dataset.py

@@ -101,7 +101,7 @@ class MarkdownDescription(SyftObject):
 
     text: str
 
-    def _repr_markdown_(self):
+    def _repr_markdown_(self) -> str:
         style = """
         <style>
             .jp-RenderedHTMLCommon pre {
@@ -138,7 +138,9 @@ class Asset(SyftObject):
     __repr_attrs__ = ["name", "shape"]
 
     def __init__(
-        self, description: Optional[Union[MarkdownDescription, str]] = "", **data
+        self,
+        description: Optional[Union[MarkdownDescription, str]] = "",
+        **data: Any,
     ):
         if isinstance(description, str):
             description = MarkdownDescription(text=description)
@@ -260,10 +262,10 @@ def mock(self) -> Union[SyftError, Any]:
         except Exception as e:
             return SyftError(message=f"Failed to get mock. {e}")
 
-    def has_data_permission(self):
+    def has_data_permission(self) -> bool:
         return self.data is not None
 
-    def has_permission(self, data_result):
+    def has_permission(self, data_result: Any) -> bool:
         # TODO: implement in a better way
         return not (
             isinstance(data_result, str)
@@ -329,7 +331,7 @@ class CreateAsset(SyftObject):
     class Config:
         validate_assignment = True
 
-    def __init__(self, description: Optional[str] = "", **data):
+    def __init__(self, description: Optional[str] = "", **data: Any) -> None:
         super().__init__(**data, description=MarkdownDescription(text=str(description)))
 
     @root_validator()
@@ -470,14 +472,16 @@ class Dataset(SyftObject):
     __repr_attrs__ = ["name", "url", "created_at"]
 
     def __init__(
-        self, description: Optional[Union[str, MarkdownDescription]] = "", **data
-    ):
+        self,
+        description: Optional[Union[str, MarkdownDescription]] = "",
+        **data: Any,
+    ) -> None:
         if isinstance(description, str):
             description = MarkdownDescription(text=description)
         super().__init__(**data, description=description)
 
     @property
-    def icon(self):
+    def icon(self) -> str:
         return FOLDER_ICON
 
     def _coll_repr_(self) -> Dict[str, Any]:
@@ -498,7 +502,7 @@ def _repr_html_(self) -> Any:
             if self.uploader
             else ""
         )
-
+        description_text: str = self.description.text if self.description else ""
         return f"""
             <style>
             {fonts_css}
@@ -510,7 +514,7 @@ def _repr_html_(self) -> Any:
             </style>
             <div class='syft-dataset'>
             <h3>{self.name}</h3>
-            <p>{self.description.text}</p>
+            <p>{description_text}</p>
             {uploaded_by_line}
             <p class='paragraph-sm'><strong><span class='pr-8'>Created on: </span></strong>{self.created_at}</p>
             <p class='paragraph-sm'><strong><span class='pr-8'>URL:
@@ -535,7 +539,10 @@ def _old_repr_markdown_(self) -> str:
         _repr_str = f"Syft Dataset: {self.name}\n"
         _repr_str += "Assets:\n"
         for asset in self.asset_list:
-            _repr_str += f"\t{asset.name}: {asset.description.text}\n"
+            if asset.description is not None:
+                _repr_str += f"\t{asset.name}: {asset.description.text}\n\n"
+            else:
+                _repr_str += f"\t{asset.name}\n\n"
         if self.citation:
             _repr_str += f"Citation: {self.citation}\n"
         if self.url:
@@ -552,7 +559,10 @@ def _markdown_(self) -> str:
         _repr_str = f"Syft Dataset: {self.name}\n\n"
         _repr_str += "Assets:\n\n"
         for asset in self.asset_list:
-            _repr_str += f"\t{asset.name}: {asset.description.text}\n\n"
+            if asset.description is not None:
+                _repr_str += f"\t{asset.name}: {asset.description.text}\n\n"
+            else:
+                _repr_str += f"\t{asset.name}\n\n"
         if self.citation:
             _repr_str += f"Citation: {self.citation}\n\n"
         if self.url:
@@ -621,7 +631,7 @@ class CreateDataset(Dataset):
 
     id: Optional[UID] = None
     created_at: Optional[DateTime]
-    uploader: Optional[Contributor]
+    uploader: Optional[Contributor]  # type: ignore[assignment]
 
     class Config:
         validate_assignment = True
@@ -670,7 +680,7 @@ def add_contributor(
             return SyftError(message=f"Failed to add contributor. Error: {e}")
 
     def add_asset(
-        self, asset: CreateAsset, force_replace=False
+        self, asset: CreateAsset, force_replace: bool = False
     ) -> Union[SyftSuccess, SyftError]:
         if asset.mock is None:
             raise ValueError(_ASSET_WITH_NONE_MOCK_ERROR_MESSAGE)
@@ -694,7 +704,7 @@ def add_asset(
             message=f"Asset '{asset.name}' added to '{self.name}' Dataset."
         )
 
-    def replace_asset(self, asset: CreateAsset):
+    def replace_asset(self, asset: CreateAsset) -> Union[SyftSuccess, SyftError]:
         return self.add_asset(asset=asset, force_replace=True)
 
     def remove_asset(self, name: str) -> None:

packages/syft/src/syft/service/dataset/dataset_service.py

@@ -2,6 +2,7 @@
 from collections.abc import Collection
 from typing import List
 from typing import Optional
+from typing import Sequence
 from typing import Union
 
 # relative
@@ -52,7 +53,7 @@ def _paginate_collection(
 
 
 def _paginate_dataset_collection(
-    datasets: Collection[Dataset],
+    datasets: Sequence[Dataset],
     page_size: Optional[int] = 0,
     page_index: Optional[int] = 0,
 ) -> Union[DictTuple[str, Dataset], DatasetPageView]:
@@ -204,7 +205,9 @@ def get_assets_by_action_id(
         name="dataset_delete_by_id",
         warning=HighSideCRUDWarning(confirmation=True),
     )
-    def delete_dataset(self, context: AuthedServiceContext, uid: UID):
+    def delete_dataset(
+        self, context: AuthedServiceContext, uid: UID
+    ) -> Union[SyftSuccess, SyftError]:
         result = self.stash.delete_by_uid(context.credentials, uid)
         if result.is_ok():
             return result.ok()