Skip to content

adding initial code for StratifiedShuffleSplit - #243

Open
VariableVasasMT wants to merge 9 commits into
machinelearnjs:masterfrom
VariableVasasMT:issues/215
Open

VariableVasasMT wants to merge 9 commits into
machinelearnjs:masterfrom
VariableVasasMT:issues/215

Conversation

@VariableVasasMT

Copy link
Copy Markdown
Collaborator
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    implementation for StratifiedShuffleSplit

Comment thread src/lib/model_selection/_split.ts Outdated
Comment thread src/lib/model_selection/_split.ts Outdated
Comment thread src/lib/model_selection/_split.ts Outdated
Comment thread src/lib/model_selection/_split.ts Outdated
@JasonShin

Copy link
Copy Markdown
Member

There is a test failure due to the doc processor cannot handle certain type. Could you please take a look at it? Let me know if it's too hard.

@JasonShin JasonShin closed this Jun 13, 2019
@JasonShin JasonShin reopened this Jun 13, 2019
@VariableVasasMT

Copy link
Copy Markdown
Collaborator Author

sure will do

Comment thread src/lib/model_selection/_split.ts Outdated
@JasonShin

Copy link
Copy Markdown
Member

Also, could you make sure these things?

  1. Export StratifiedShuffleSplit in index.ts (check model_selection/index.ts) so the users can actually use it
  2. add an integration test
  3. Please add unit tests to ensure 100% coverage and all the behaviours for this code
  4. You need to add doc strings (check other APIs) so the doc processor can actually pick it up and generate documentation.

Comment thread src/lib/model_selection/_split.ts Outdated
Comment thread src/lib/model_selection/_split.ts Outdated
Comment thread src/lib/model_selection/_split.ts Outdated
Comment thread src/lib/model_selection/_split.ts Outdated
  * added arraySplit
  * moved forward in stratifiedShuffleSplit
 * added random class
 * adding StratifiedShuffleSplit
 * adding approximateMode, cumSum, countBin in tensors.ts
@VariableVasasMT

Copy link
Copy Markdown
Collaborator Author

Also, could you make sure these things?

1. Export `StratifiedShuffleSplit` in `index.ts` (check model_selection/index.ts) so the users can actually use it

2. add an integration test

3. Please add unit tests to ensure 100% coverage and all the behaviours for this code

4. You need to add doc strings (check other APIs) so the doc processor can actually pick it up and generate documentation.

fixed

@VariableVasasMT

Copy link
Copy Markdown
Collaborator Author

@JasonShin please review this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants