red-data-tools / unicode_plot.rb

Plot your data by Unicode characters
MIT License
246 stars 12 forks source link

Idea: A "ruby only" core version of this library #31

Closed schneems closed 3 years ago

schneems commented 3 years ago

Hello again! Thanks a ton for this library (again). I've got another bit of a question. Would you consider porting over this codebase to not rely on enumerable-statistics?

Here's my context:

Example Proposal

I took the liberty of forking this lib to see what removing enumerable-statistics would look like. This ended up being the commit:

https://github.com/zombocom/mini_unicode_plot/commit/18c0242f653a856374a6b6cacbf90a2db6027387

Essentially we can get rid of this dependency by only removing two plot types (box plot and histogram plot). Before I fork and try to maintain my own plotting library that duplicates all this functionality. I wanted to ask if you had any interest in porting over unicode_plot to have some kind of "ruby only" core and then keep this library to have extensions with enumerable-statistics? I don't want to duplicate your efforts if possible.

The other change I would like to make is to increase backward compatibility to Ruby 2.2. This would require a few cases of not using &. syntax and porting over the tests to circle-ci as GitHub-actions do not have a Ruby version that goes back that far.

What do you think?

mrkn commented 3 years ago

Supporting Ruby 2.2 is good to me. Could you make a pull-request that consists of only changes related to increase backward compatibility?

Depending on the different libraries for single feature is not good. I think what we should do is not making mini_histogram, but letting enumerable-statistics support platforms other than CRuby. I will prioritize the work to do that, so please give me some time.

schneems commented 3 years ago

Supporting Ruby 2.2 is good to me. Could you make a pull-request that consists of only changes related to increase backward compatibility?

The only thing I needed to do was move away from using &. calls. Here's an example https://github.com/zombocom/mini_histogram/commit/e17782d7f11976073dd47e8cc03d38ae90ad5ec3. But enumerable-statistics is still locked ro Ruby 2.4+ which is the bigger issue preventing supporting older versions. I do not know what difficulties enumerable-statistics face to support older Rubies.

mrkn commented 3 years ago

@schneems OK. I’ll try to let enumerable-statistics support Ruby 2.2, too.

schneems commented 3 years ago

@schneems OK. I’ll try to let enumerable-statistics support Ruby 2.2, too.

That is exciting! It sounds like a large project. Thank you! For running on CI, Puma also supports 2.2. Here's their Github actions https://github.com/puma/puma/blob/master/.github/workflows/puma.yml#L28-L31

I think I can close this proposal as "not accepted" but it sounds like you have action items generated from this discussion.