Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

CorpusReader #10061

Merged
merged 3 commits into from Mar 10, 2018
Merged

CorpusReader #10061

merged 3 commits into from Mar 10, 2018

Conversation

szha
Copy link
Member

@szha szha commented Mar 10, 2018

Description

Add CorpusReader.

Checklist

Essentials

  • Passed code style checking (make lint)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Refactor the reading and tokenization logic from WordLanguageReader into CorpusReader.

Comments

  • sentence boundary is not respected.

Copy link
Member

@sxjscience sxjscience left a comment

Choose a reason for hiding this comment

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

We will keep in mind the 80-20 rule when design the NLP APIs.

@sxjscience sxjscience merged commit 453fc23 into apache:nlp_toolkit Mar 10, 2018
szha added a commit to szha/mxnet that referenced this pull request Mar 15, 2018
* corpus reader

* update

* delete pair
szha added a commit to szha/mxnet that referenced this pull request Mar 15, 2018
* corpus reader

* update

* delete pair
@szha szha deleted the sent_reader branch April 26, 2018 18:21
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.

None yet

2 participants