Skip to content
This repository was archived by the owner on Dec 16, 2022. It is now read-only.

Conversation

@OhadRubin
Copy link
Contributor

@OhadRubin OhadRubin commented Aug 27, 2020

#4591
This is a really rough draft...

@OhadRubin
Copy link
Contributor Author

Hey, @epwalsh, i'm in the process of adapting the original fairseq code to our needs.
Right now the structure of the classes if kinda weird with _Writer and Index (from fairseq).
Can you review what I have here so far?

@OhadRubin OhadRubin marked this pull request as ready for review August 31, 2020 08:53
@epwalsh
Copy link
Member

epwalsh commented Aug 31, 2020

Hey @OhadRubin, is there any reason why the Index class needs to be defined within the MMapCacheReader class?

@epwalsh epwalsh self-assigned this Aug 31, 2020
@OhadRubin
Copy link
Contributor Author

No, that's one of the things from fairseq I kinda want to change...

@OhadRubin
Copy link
Contributor Author

OhadRubin commented Aug 31, 2020

Btw, during instance creation, is it possible to require that all fields exists, this will really help deserialization.
Where in "test set" situation, where there isn't any supervision, the user passes a empty field, or something along that.
This will allow us to use things that expect a certain structure (schema) like pyarrow here

@schmmd schmmd changed the base branch from master to main December 23, 2020 18:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants