markrogoyski / itertools-php

PHP Iteration Tools Library
MIT License
140 stars 11 forks source link

Method `Single::frequencies()` added #48

Closed Smoren closed 1 year ago

Smoren commented 1 year ago

Hi @markrogoyski,

This is a draft of PR. If you like my implementation, I'll finish it.

BTW I think this method is more suitable for Single namespace, not for Reduce (as you suggest) because it returns a generator.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4465593691

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Totals Coverage Status
Change from base Build 4462214447: 0.002%
Covered Lines: 787
Relevant Lines: 788

💛 - Coveralls
Smoren commented 1 year ago

Hi @markrogoyski, thank you, I used zipEqual() as you suggested.

Also I added Stream::frequencies() method and covered it with test.

BTW, I what do you think about a new namespace like Stat for this function? Or maybe use Math?

markrogoyski commented 1 year ago

Hi @Smoren,

Thank you for this pull request, and apologies at the delay at reviewing it.

I have a suggestion, let's create two functions, using the technical definitions:

For reference, this is also the same terminology used in MathPHP.

use MathPHP\Statistics\Distribution;

$grades = ['A', 'A', 'B', 'B', 'B', 'B', 'C', 'C', 'D', 'F'];

// Frequency distributions (frequency and relative frequency)
$frequencies          = Distribution::frequency($grades);         // [ A => 2,   B => 4,   C => 2,   D => 1,   F => 1   ]
$relative_frequencies = Distribution::relativeFrequency($grades); // [ A => 0.2, B => 0.4, C => 0.2, D => 0.1, F => 0.1 ]

Thoughts about this change? Thanks, Mark

Smoren commented 1 year ago

Hi @markrogoyski,

Your suggestion is implemented!

markrogoyski commented 1 year ago

Hi @Smoren,

Thanks for implementing the change. Looks good to me.

Mark

Smoren commented 1 year ago

Thank you @markrogoyski!

markrogoyski commented 1 year ago

Hi @Smoren,

Considering one of your earlier comments a bit more, I moved this functionality into the Math namespace. It seems a better fit there.

Thanks again, Mark

Smoren commented 1 year ago

Thank you @markrogoyski!