New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure DataTransformer::testSet samples array is not empty #204

Merged
merged 1 commit into from Feb 25, 2018

Conversation

Projects
None yet
2 participants
@marmichalski
Contributor

marmichalski commented Jan 26, 2018

No description provided.

@@ -24,6 +26,10 @@ public static function trainingSet(array $samples, array $labels, bool $targets
public static function testSet(array $samples): string
{
if (!isset($samples[0])) {

This comment has been minimized.

@akondas

akondas Jan 26, 2018

Contributor

hmm, you can pass [1 => [], 2 => []], so maybe better to check empty

This comment has been minimized.

@marmichalski

marmichalski Jan 26, 2018

Contributor

In that case this line https://github.com/php-ai/php-ml/blob/master/src/Phpml/SupportVectorMachine/DataTransformer.php#L27 will also fail.

Shouldn't we check all $samples elements and convert them to array, if they're not?
Passing [[1], [2], 'abc'] will make ::sampleRow to fail, as argument will be a string instead of expected array and by passing ['abc', [1], [2]] libsvm will fail with wrong input format. Same happens with nested arrays: [[1, [2]]].

This comment has been minimized.

@akondas

akondas Jan 29, 2018

Contributor

This class is internal for SVM algorithm. I think we can protect libsvm from fail. So to answer your question better to make php fail :)

@marmichalski marmichalski force-pushed the marmichalski:data-transformer-arg branch from 1333baf to 7d390b8 Feb 15, 2018

@akondas akondas merged commit 9e375ca into php-ai:master Feb 25, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@marmichalski marmichalski deleted the marmichalski:data-transformer-arg branch Mar 4, 2018

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