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

IOMixin for DocVec (to/from) #1330

Closed
jupyterjazz opened this issue Apr 3, 2023 · 4 comments · Fixed by #1562
Closed

IOMixin for DocVec (to/from) #1330

jupyterjazz opened this issue Apr 3, 2023 · 4 comments · Fixed by #1562
Assignees

Comments

@jupyterjazz
Copy link
Contributor

jupyterjazz commented Apr 3, 2023

We want to support IO functionality in DocVec such as from/to json, csv, bytes, etc. For example, to/from json is needed to make it FastAPI-compatible.

However, we can not use the existing IOMixin here as the logic behind DocVec is different from DocList and we'd have to overwrite some functions.

Solution:
Create an abstract mixin containing functions that the two classes share, and for other functions create separate mixins inheriting from the abstract class

@JoanFM JoanFM changed the title IOMixin for DocArrayStacked IOMixin for Doc2Vec (to/from) Apr 27, 2023
@JoanFM
Copy link
Member

JoanFM commented Apr 27, 2023

The description needs to be changed to Doc2Vec

@JoanFM
Copy link
Member

JoanFM commented May 10, 2023

When implementing this, please make sure to habdle the case shown in #1490

@JoanFM JoanFM changed the title IOMixin for Doc2Vec (to/from) IOMixin for DocVec (to/from) May 19, 2023
@JohannesMessner
Copy link
Member

JohannesMessner commented May 22, 2023

I am breaking this up into multiple PRs (will update the list as I go):

@samsja
Copy link
Member

samsja commented Jun 2, 2023

lets not do too much abstraction here. There are no shared behavior between DocList and DocVec IO. Lets juts implement the function in both, no need for an abstract class until we can provide the same IO capacity for both

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants