GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat(v2): load da from csv and save to csv by anna-charlotte · Pull Request #1144 · docarray/docarray

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

Conversation

@anna-charlotte

@anna-charlotte anna-charlotte commented Feb 17, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@anna-charlotte 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 github-actions Bot added size/s area/core area/testing component/array labels Feb 17, 2023
@anna-charlotte anna-charlotte mentioned this pull request Feb 20, 2023
Meta: DocArray v2 Roadmap #780
Closed
42 of 47 tasks
@JohannesMessner

JohannesMessner commented Feb 20, 2023 •
edited by anna-charlotte
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Will nested schemas work somehow?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anna-charlotte

anna-charlotte commented Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anna-charlotte
anna-charlotte force-pushed the feat-from-to-csv branch from 7f03c5e to ed134c5 Compare February 20, 2023 09:34
@github-actions github-actions Bot added size/m and removed size/s labels Feb 20, 2023
@anna-charlotte
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
anna-charlotte
feat: load from and to csv
dce8c6b 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: from to csv
37892a9 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
feat: add access path to dict
e1e90cf 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: from to csv
7bd7f1f 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: clean up
05e19ba 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
docs: add docstring and update tmpdir in test
5b0c760 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: merge nested dicts
5babdae 
Signed-off-by: anna-charlotte <[email protected]>
@anna-charlotte
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
anna-charlotte
fix: clean up
d6b29c5 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: clean up
9149e57 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
test: update test
3d980b7 
Signed-off-by: anna-charlotte <[email protected]>
@anna-charlotte
anna-charlotte marked this pull request as ready for review February 20, 2023 14:49
@anna-charlotte
anna-charlotte requested review from JohannesMessner and samsja February 20, 2023 14:49
samsja
samsja reviewed Feb 20, 2023
View reviewed changes
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
samsja
samsja reviewed Feb 20, 2023
View reviewed changes
Comment thread tests/units/array/test_array_from_to_csv.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
samsja
samsja reviewed Feb 20, 2023
View reviewed changes
Comment thread tests/toydata/docs_nested.csv Outdated
@@ -0,0 +1,4 @@
count,text,image,image2.url

@samsja samsja Feb 20, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anna-charlotte anna-charlotte Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, do you mean like by csv convention?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@samsja samsja Feb 20, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anna-charlotte anna-charlotte Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, will adjust this

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anna-charlotte anna-charlotte Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
samsja
samsja reviewed Feb 20, 2023
View reviewed changes
Comment thread 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'))

@samsja samsja Feb 20, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anna-charlotte anna-charlotte Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@samsja samsja Feb 20, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

yes makes sense to do it in another PR

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
anna-charlotte
fix: apply samis suggestion from code review
516ffbb 
Signed-off-by: anna-charlotte <[email protected]>
JohannesMessner
JohannesMessner suggested changes Feb 21, 2023
View reviewed changes

@JohannesMessner JohannesMessner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docarray/array/array/io.py Outdated
"""
Check if a given access path is a valid path for a given Document class.
"""
field, _, remaining = access_path.partition('__')

@JohannesMessner JohannesMessner Feb 21, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anna-charlotte anna-charlotte Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JohannesMessner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread docarray/array/array/io.py Outdated
Comment on lines +622 to +682
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

@JohannesMessner JohannesMessner Feb 21, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
samsja reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@anna-charlotte anna-charlotte Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@JohannesMessner JohannesMessner Feb 21, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anna-charlotte anna-charlotte Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ok done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Charlotte Gerhaher and others added 7 commits February 21, 2023 13:29
Charlotte Gerhaher
Merge branch 'feat-rewrite-v2' into feat-from-to-csv
5fac964
anna-charlotte
fix: apply suggestions from code review wrt access paths
a0d9711 
Signed-off-by: anna-charlotte <[email protected]>
Charlotte Gerhaher @JohannesMessner
fix: apply johannes suggestion
c9005b1 
Co-authored-by: Johannes Messner <[email protected]>
Signed-off-by: Charlotte Gerhaher <[email protected]>
Charlotte Gerhaher @JohannesMessner
fix: apply johannes suggestion
9fe58f5 
Co-authored-by: Johannes Messner <[email protected]>
Signed-off-by: Charlotte Gerhaher <[email protected]>
anna-charlotte
fix: apply suggestions from code review
90867bb 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: apply suggestions from code review
1a395da 
Signed-off-by: anna-charlotte <[email protected]>
anna-charlotte
fix: typos
00a9ea7 
Signed-off-by: anna-charlotte <[email protected]>
@anna-charlotte
anna-charlotte force-pushed the feat-from-to-csv branch from bc46938 to 00a9ea7 Compare February 21, 2023 12:31
anna-charlotte
refactor: move helper functions to helper file
e06e533 
Signed-off-by: anna-charlotte <[email protected]>
@anna-charlotte
anna-charlotte requested review from JohannesMessner and samsja February 21, 2023 15:25
anna-charlotte
test: fix fixture
c8e4cf8 
Signed-off-by: anna-charlotte <[email protected]>
samsja
samsja approved these changes Feb 21, 2023
View reviewed changes
Charlotte Gerhaher
Merge branch 'feat-rewrite-v2' into feat-from-to-csv
d0a3141
@github-actions

github-actions Bot commented Feb 21, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JohannesMessner
JohannesMessner approved these changes Feb 22, 2023
View reviewed changes

@JohannesMessner JohannesMessner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nice feature!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@anna-charlotte
anna-charlotte merged commit 251eea7 into feat-rewrite-v2 Feb 22, 2023
@anna-charlotte
anna-charlotte deleted the feat-from-to-csv branch February 22, 2023 13:15
@anna-charlotte 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
AsRaNi1 pushed a commit to AsRaNi1/docarray that referenced this pull request Feb 28, 2023
Charlotte Gerhaher @JohannesMessner @AsRaNi1
feat(v2): load da from csv and save to csv (docarray#1144)
cdbc117 
* 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]>
@JohannesMessner JohannesMessner mentioned this pull request Mar 1, 2023
Dynamic class creation by "guessing" types #1191
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@JohannesMessner JohannesMessner JohannesMessner approved these changes
@samsja samsja samsja approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

area/core area/testing component/array size/m

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement from_to/csv,pandas v2: handle load from external data sources

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@anna-charlotte @JohannesMessner @samsja
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.