Remove non-base datasets. #1275
Conversation
|
looks ok but @bpopeters can you please review. |
| if hasattr(ex, "tgt"): | ||
| return len(ex.src[0]), len(ex.tgt[0]) | ||
| return len(ex.src[0]) | ||
| def text_sort_key(ex): |
There was a problem hiding this comment.
The style of defining sort_key as a static method of a dataset class was to match torchtext, where I think the general idea is people would define a different dataset class for each dataset (classes with names like IWSLT and WMT14, for example). But for the way we're doing it, with just the single Dataset class, I agree that this makes sense.
|
Is this waiting on anything? |
|
I was about to merge it but Travis is weird. |
|
@flauted this breaks pre-existing pre-processed files, not a big deal but in case this could be fixed. |
|
You bumped the major version so I'll let it be. But for what it's worth fixing it would I think require leaving stub |
|
yeah that's what I figured out. |
* Remove non-base datasets. * Update Dataset documentation. * Move helper methods out of Dataset and document them. * Remove , replace w explicit constructor calls.
The various [Datatype]Dataset's are just shells that define a
sort_keystatic method. Sincetorchtext.data.Datasetonly uses thesort_keyattribute attached to the instanceself(despite defining it at the class level) , we can just make thesort_keyan instance attribute of theDatasetBase. ConsequentlyDatasetBaseis now a generalDataset. After this PR, the file dataset_base.py should be renamed to dataset.py.