Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

feat: use literal in type hints #1827

Merged
JoanFM merged 6 commits into from
Oct 24, 2023
Merged

feat: use literal in type hints #1827

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
784fc40
feat(types): added ProtocolType to give more constrained type to `pro…
bpshaver Oct 23, 2023
9dc0eff
feat(types): expanded set of allowed protocol types
bpshaver Oct 23, 2023
1977708
refactor(types): changed instances of protocol parameter to use new P…
bpshaver Oct 23, 2023
61eb05f
chore(fmt): auto-formatting
bpshaver Oct 23, 2023
d60ebe6
fix(types): moving import ProtocolType from under TYPE_CHECKING box
bpshaver Oct 23, 2023
bbb8939
fix(types): moving import of ProtocolType from under TYPE_CHECKING block
bpshaver Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore(fmt): auto-formatting 
Signed-off-by: Ben Shaver <benpshaver@gmail.com>
  • Loading branch information
@bpshaver
bpshaver committed Oct 23, 2023
commit 61eb05f41dbaac96b9d64f7d3bdd6f8c762d0157
9 changes: 6 additions & 3 deletions docarray/array/doc_list/io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ def _from_csv_file(
file: Union[StringIO, TextIOWrapper],
dialect: Union[str, csv.Dialect],
) -> 'T':

rows = csv.DictReader(file, dialect=dialect)

doc_type = cls.doc_type
Expand Down Expand Up @@ -659,7 +658,9 @@ def _load_binary_all(
start_pos = end_doc_pos

# variable length bytes doc
load_protocol: ProtocolType = protocol or cast(ProtocolType, 'protobuf')
load_protocol: ProtocolType = protocol or cast(
ProtocolType, 'protobuf'
)
doc = cls.doc_type.from_bytes(
d[start_doc_pos:end_doc_pos],
protocol=load_protocol,
Expand Down Expand Up @@ -745,7 +746,9 @@ def _get_file_context(
file: Union[str, bytes, pathlib.Path, io.BufferedReader, _LazyRequestReader],
protocol: ProtocolType,
compress: Optional[str] = None,
) -> Tuple[Union[nullcontext, io.BufferedReader], Optional[ProtocolType], Optional[str]]:
) -> Tuple[
Union[nullcontext, io.BufferedReader], Optional[ProtocolType], Optional[str]
]:
load_protocol: Optional[ProtocolType] = protocol
load_compress: Optional[str] = compress
file_ctx: Union[nullcontext, io.BufferedReader]
Expand Down
1 change: 0 additions & 1 deletion docarray/array/doc_vec/io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ def _from_json_col_dict(
json_columns: Dict[str, Any],
tensor_type: Type[AbstractTensor] = NdArray,
) -> T:

tensor_cols = json_columns['tensor_columns']
doc_cols = json_columns['doc_columns']
docs_vec_cols = json_columns['docs_vec_columns']
Expand Down
2 changes: 0 additions & 2 deletions docarray/base_doc/mixins/io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ def _get_content_from_node_proto(
return_field = getattr(value, content_key)

elif content_key in arg_to_container.keys():

if field_name and field_name in cls._docarray_fields():
field_type = cls._get_field_inner_type(field_name)
else:
Expand All @@ -347,7 +346,6 @@ def _get_content_from_node_proto(
deser_dict: Dict[str, Any] = dict()

if field_name and field_name in cls._docarray_fields():

if is_pydantic_v2:
dict_args = get_args(
cls._docarray_fields()[field_name].annotation
Expand Down
5 changes: 4 additions & 1 deletion docarray/utils/_internal/misc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@
'pymilvus': '"docarray[milvus]"',
}

ProtocolType = Literal['protobuf', 'pickle', 'json', 'json-array', 'protobuf-array', 'pickle-array']
ProtocolType = Literal[

@bpshaver bpshaver Oct 23, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Followed the ALLOWED_PROTOCOLS constant at https://github.com/docarray/docarray/blob/main/docarray/array/doc_list/io.py#L54C1-L54C1

It makes me uncomfortable to define the same information twice, so I can investigate combining the ProtocolType and ALLOWED_PROTOCOLS objects, even though one is a type object and the other is a set of strings.

@JoanFM JoanFM Oct 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In a follow-up PR you can define ALLOWED_PROTOCOLS in relation to these types

'protobuf', 'pickle', 'json', 'json-array', 'protobuf-array', 'pickle-array'
]


def import_library(
package: str, raise_error: bool = True
Expand Down