feat(v2): load da from csv and save to csv #1144

anna-charlotte · 2023-02-17T10:02:33Z

Goals:

load DocumentArray from csv, with a given Document class to provide a schema
- document_type needs to be set to determine schema.
- column names have to match the Document types fields
- for nested fields, use access paths as column name, such as 'image__url'
- if value is None, leave csvfield empty, or enter None
save a DocumentArray to csv
- following the same guidelines as above.
check and update documentation, if required. See guide

JohannesMessner · 2023-02-20T08:33:34Z

Will nested schemas work somehow?

anna-charlotte · 2023-02-20T08:54:55Z

Will nested schemas work somehow?

Yes, for nested fields you can give the access path as column names, e.g. 'image.url' to set the url of a field of type Image

Signed-off-by: anna-charlotte <[email protected]>

docarray/array/array/io.py

tests/units/array/test_array_from_to_csv.py

samsja · 2023-02-20T15:42:26Z

tests/toydata/docs_nested.csv

@@ -0,0 +1,4 @@
+count,text,image,image2.url


Suggested change

count,text,image,image2.url

count,text,image,image2__url

Not really sure here but I think we cannot/should not use . inside column name

Ok, do you mean like by csv convention?

Yeah kind of because, though it is more gut feeling that actual knowledge about CSV convention. But generally I think we should keep with a__b sintax rather than a.b one. Same for vector database I believe @JohannesMessner

Ok, will adjust this

Didn't we use "dot-notation" in other places, e.g. nested.field instead of nested__field? We need to make sure that this is uniform everywhere.

@JohannesMessner true, at other places we used the dot separation for the access paths (such as in traverse_flat(), e.g.da.traverse_flat(access_path='author.name'). @samsja mentioned that we should not use the . inside the column name. I am not sure how dangerous/against the rules this is

samsja · 2023-02-20T15:43:41Z

tests/units/array/test_array_from_to_csv.py

+
+def test_from_csv_without_schema_raise_exception():
+    with pytest.raises(TypeError, match='no document schema defined'):
+        DocumentArray.from_csv(file_path=str(TOYDATA_DIR / 'docs_nested.csv'))


Makes sense that this raise an Expection for now. But what about implementing at this level the auto schema detection ?

Yes, for now the exception, I think the auto detection or handing over a schema some other way then setting .document_type I would handle in a separate PR, if that's good with you.

yes makes sense to do it in another PR

Signed-off-by: anna-charlotte <[email protected]>

JohannesMessner

Some comments, but I think this will be a truly cool feature! We should promote this properly once it is out, I bet the data validation folks that already use pydantic would love it

docarray/array/array/io.py

JohannesMessner · 2023-02-21T08:41:48Z

docarray/array/array/io.py

+    """
+    Check if a given access path is a valid path for a given Document class.
+    """
+    field, _, remaining = access_path.partition('__')


Didn't we use "dot-notation" in other places, e.g. nested.field instead of nested__field? We need to make sure that this is uniform everywhere.

Update: we will use __ instead of . separator for access paths

JohannesMessner · 2023-02-21T08:44:07Z

docarray/array/array/io.py

+def _access_path_to_dict(access_path: str, value) -> Dict[str, Any]:
+    """
+    Convert an access path and its value to a (potentially) nested dict.
+
+    EXAMPLE USAGE
+    .. code-block:: python
+        assert access_path_to_dict('image__url', 'img.png') == {'image': {'url': 'img.png'}}
+    """
+    fields = access_path.split('__')
+    for field in reversed(fields):
+        result = {field: value}
+        value = result
+    return result
+
+
+def _dict_to_access_paths(d: dict) -> Dict[str, Any]:
+    """
+    Convert a (nested) dict to a Dict[access_path, value].
+    Access paths are defines as a path of field(s) separated by "__".
+
+    EXAMPLE USAGE
+    .. code-block:: python
+        assert dict_to_access_paths({'image': {'url': 'img.png'}}) == {'image__url', 'img.png'}
+    """
+    result = {}
+    for k, v in d.items():
+        if isinstance(v, dict):
+            v = _dict_to_access_paths(v)
+            for nested_k, nested_v in v.items():
+                new_key = '__'.join([k, nested_k])
+                result[new_key] = nested_v
+        else:
+            result[k] = v
+    return result


We should consider making these methods global helpers, they will come in handy in other places of the code base as well, e.g. i already implemented something similar for document stores

yes, sure, sounds good. What exactly do you mean by making these methods global helpers, put them somewhere in a helper file?

yes exactly, in a helper.py or utils.py at the top level or something like that

Signed-off-by: anna-charlotte <[email protected]>

Co-authored-by: Johannes Messner <[email protected]> Signed-off-by: Charlotte Gerhaher <[email protected]>

Signed-off-by: anna-charlotte <[email protected]>

github-actions · 2023-02-21T16:17:56Z

📝 Docs are deployed on https://ft-feat-from-to-csv--jina-docs.netlify.app 🎉

JohannesMessner

Nice feature!

* feat: load from and to csv Signed-off-by: anna-charlotte <[email protected]> * fix: from to csv Signed-off-by: anna-charlotte <[email protected]> * feat: add access path to dict Signed-off-by: anna-charlotte <[email protected]> * fix: from to csv Signed-off-by: anna-charlotte <[email protected]> * fix: clean up Signed-off-by: anna-charlotte <[email protected]> * docs: add docstring and update tmpdir in test Signed-off-by: anna-charlotte <[email protected]> * fix: merge nested dicts Signed-off-by: anna-charlotte <[email protected]> * fix: clean up Signed-off-by: anna-charlotte <[email protected]> * fix: clean up Signed-off-by: anna-charlotte <[email protected]> * test: update test Signed-off-by: anna-charlotte <[email protected]> * fix: apply samis suggestion from code review Signed-off-by: anna-charlotte <[email protected]> * fix: apply suggestions from code review wrt access paths Signed-off-by: anna-charlotte <[email protected]> * fix: apply johannes suggestion Co-authored-by: Johannes Messner <[email protected]> Signed-off-by: Charlotte Gerhaher <[email protected]> * fix: apply johannes suggestion Co-authored-by: Johannes Messner <[email protected]> Signed-off-by: Charlotte Gerhaher <[email protected]> * fix: apply suggestions from code review Signed-off-by: anna-charlotte <[email protected]> * fix: apply suggestions from code review Signed-off-by: anna-charlotte <[email protected]> * fix: typos Signed-off-by: anna-charlotte <[email protected]> * refactor: move helper functions to helper file Signed-off-by: anna-charlotte <[email protected]> * test: fix fixture Signed-off-by: anna-charlotte <[email protected]> --------- Signed-off-by: anna-charlotte <[email protected]> Signed-off-by: Charlotte Gerhaher <[email protected]> Co-authored-by: Johannes Messner <[email protected]> Signed-off-by: Arnav Zutshi <[email protected]>

anna-charlotte linked an issue Feb 17, 2023 that may be closed by this pull request

Implement from_to/csv,pandas #1126

Closed

github-actions bot added size/s area/core area/testing component/array labels Feb 17, 2023

anna-charlotte mentioned this pull request Feb 20, 2023

Meta: DocArray v2 Roadmap #780

Closed

47 tasks

anna-charlotte force-pushed the feat-from-to-csv branch from 7f03c5e to ed134c5 Compare February 20, 2023 09:34

github-actions bot added size/m and removed size/s labels Feb 20, 2023

anna-charlotte force-pushed the feat-from-to-csv branch from b21d518 to 5126056 Compare February 20, 2023 12:33

anna-charlotte added 7 commits February 20, 2023 15:06

feat: load from and to csv

dce8c6b

Signed-off-by: anna-charlotte <[email protected]>

fix: from to csv

37892a9

Signed-off-by: anna-charlotte <[email protected]>

feat: add access path to dict

e1e90cf

Signed-off-by: anna-charlotte <[email protected]>

fix: from to csv

7bd7f1f

Signed-off-by: anna-charlotte <[email protected]>

fix: clean up

05e19ba

Signed-off-by: anna-charlotte <[email protected]>

docs: add docstring and update tmpdir in test

5b0c760

Signed-off-by: anna-charlotte <[email protected]>

fix: merge nested dicts

5babdae

Signed-off-by: anna-charlotte <[email protected]>

anna-charlotte force-pushed the feat-from-to-csv branch from 27303af to 5babdae Compare February 20, 2023 14:07

anna-charlotte added 3 commits February 20, 2023 15:28

fix: clean up

d6b29c5

Signed-off-by: anna-charlotte <[email protected]>

fix: clean up

9149e57

Signed-off-by: anna-charlotte <[email protected]>

test: update test

3d980b7

Signed-off-by: anna-charlotte <[email protected]>

anna-charlotte marked this pull request as ready for review February 20, 2023 14:49

anna-charlotte requested review from JohannesMessner and samsja February 20, 2023 14:49

samsja reviewed Feb 20, 2023

View reviewed changes

docarray/array/array/io.py Outdated Show resolved Hide resolved

samsja reviewed Feb 20, 2023

View reviewed changes

tests/units/array/test_array_from_to_csv.py Outdated Show resolved Hide resolved

samsja reviewed Feb 20, 2023

View reviewed changes

fix: apply samis suggestion from code review

516ffbb

Signed-off-by: anna-charlotte <[email protected]>

JohannesMessner requested changes Feb 21, 2023

View reviewed changes

Charlotte Gerhaher and others added 7 commits February 21, 2023 13:29

Merge branch 'feat-rewrite-v2' into feat-from-to-csv

5fac964

fix: apply suggestions from code review wrt access paths

a0d9711

Signed-off-by: anna-charlotte <[email protected]>

fix: apply johannes suggestion

c9005b1

Co-authored-by: Johannes Messner <[email protected]> Signed-off-by: Charlotte Gerhaher <[email protected]>

fix: apply johannes suggestion

9fe58f5

Co-authored-by: Johannes Messner <[email protected]> Signed-off-by: Charlotte Gerhaher <[email protected]>

fix: apply suggestions from code review

90867bb

Signed-off-by: anna-charlotte <[email protected]>

fix: apply suggestions from code review

1a395da

Signed-off-by: anna-charlotte <[email protected]>

fix: typos

00a9ea7

Signed-off-by: anna-charlotte <[email protected]>

anna-charlotte force-pushed the feat-from-to-csv branch from bc46938 to 00a9ea7 Compare February 21, 2023 12:31

refactor: move helper functions to helper file

e06e533

Signed-off-by: anna-charlotte <[email protected]>

anna-charlotte requested review from JohannesMessner and samsja February 21, 2023 15:25

test: fix fixture

c8e4cf8

Signed-off-by: anna-charlotte <[email protected]>

samsja approved these changes Feb 21, 2023

View reviewed changes

Merge branch 'feat-rewrite-v2' into feat-from-to-csv

d0a3141

JohannesMessner approved these changes Feb 22, 2023

View reviewed changes

anna-charlotte merged commit 251eea7 into feat-rewrite-v2 Feb 22, 2023

anna-charlotte deleted the feat-from-to-csv branch February 22, 2023 13:15

anna-charlotte linked an issue Feb 23, 2023 that may be closed by this pull request

v2: handle load from external data sources #971

Closed

JohannesMessner mentioned this pull request Mar 1, 2023

Dynamic class creation by "guessing" types #1191

Closed

feat(v2): load da from csv and save to csv #1144

feat(v2): load da from csv and save to csv #1144

Uh oh!

Conversation

anna-charlotte commented Feb 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohannesMessner commented Feb 20, 2023 • edited by anna-charlotte Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anna-charlotte commented Feb 20, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohannesMessner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 21, 2023

Uh oh!

JohannesMessner left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anna-charlotte commented Feb 17, 2023 •

edited

Loading

JohannesMessner commented Feb 20, 2023 •

edited by anna-charlotte

Loading