Closes #5 | Add tatoeba dataset loader #22
Conversation
ljvmiranda921
requested review from
holylovenia,
SamuelCahyawijaya,
fajri91 and
afaji
as code owners
SamuelCahyawijaya
requested review from
RosenZhang,
jcblaisecruz02 and
ryanignatius
and removed request for
RosenZhang and
jcblaisecruz02
About this, I think it will be better to "override" this with the actual language code, |
|
@ryanignatius I agree with you. Could you please override it with the actual language codes? @ljvmiranda921 |
Should be done: aba9c65 |
There was a problem hiding this comment.
One last thing, could you also please provide tatoeba_source and tatoeba_seacrowd_t2t schemas where they load all data? @ljvmiranda921
| _SEACROWD_VERSION = "1.0.0" | ||
|
|
||
|
|
||
| class TatoebaDatset(datasets.GeneratorBasedBuilder): |
There was a problem hiding this comment.
Could you please change the class name to TatoebaDataset?
There was a problem hiding this comment.
Thanks for catching! Fixed d5bef23
|
|
||
| SEACROWD_SCHEMA_NAME = "t2t" | ||
|
|
||
| dataset_names = sorted([f"tatoeba.{lang}" for lang in _LANGUAGES]) |
There was a problem hiding this comment.
Instead of using . as the delimiter, could you please change it to _?
There was a problem hiding this comment.
Sure! e95f83e
|
Converting this to draft for now. Will still work on the schema that loads all datasets at once. |
ljvmiranda921
force-pushed
the
add/tatoeba
branch
from
9317dea to
e95f83e
Compare
ljvmiranda921
force-pushed
the
add/tatoeba
branch
from
a434bfa to
a44be2b
Compare
Hi @holylovenia ! I implemented it here: a44be2b The default config now is |
There was a problem hiding this comment.
Thank you so much for the changes, @ljvmiranda921! Looks perfect to me. 👍 Let's wait for @ryanignatius's review then we can merge it.
There was a problem hiding this comment.
it looks good to me too,
thank you for the help @ljvmiranda921
Closes #5
Some notable things to point out from this dataset:
tatoeba.ind_seacrowd_t2t,tatoeba.ind_sourceand so on. When testing, we should pass a value to the--subset_idparameter like so:source_langfield in the original HF link seems incorrect? I'm not familiar with this dataset so for my implementation I just copied over what's in the source. Let me know if I should "override" this with the actual language code.Checkbox
seacrowd/sea_datasets/my_dataset/my_dataset.py(please use only lowercase and underscore for dataset naming)._CITATION,_DATASETNAME,_DESCRIPTION,_HOMEPAGE,_LICENSE,_URLs,_SUPPORTED_TASKS,_SOURCE_VERSION, and_SEACROWD_VERSIONvariables._info(),_split_generators()and_generate_examples()in dataloader script.BUILDER_CONFIGSclass attribute is a list with at least oneSEACrowdConfigfor the source schema and one for a seacrowd schema.datasets.load_datasetfunction.python -m tests.test_seacrowd seacrowd/sea_datasets/<my_dataset>/<my_dataset>.py.