spatie / period

Complex period comparisons
https://spatie.be/open-source
MIT License
1.59k stars 72 forks source link

Visualization helper #10

Closed thecrypticace closed 5 years ago

thecrypticace commented 5 years ago

See https://github.com/spatie/period/issues/7

This feature allows one to visualize periods and collections like so:

$visualizer = new Visualizer(["width" => 27]);
$visualizer->visualize([
    "A" => Period::make('2018-01-01', '2018-01-31'),
    "B" => Period::make('2018-02-10', '2018-02-20'),
    "C" => Period::make('2018-03-01', '2018-03-31'),
    "D" => Period::make('2018-01-20', '2018-03-10'),
    "OVERLAP" => new PeriodCollection(
        Period::make('2018-01-20', '2018-01-31'),
        Period::make('2018-02-10', '2018-02-20'),
        Period::make('2018-03-01', '2018-03-10')
    ),
]);

And you'll get the following string as $output:

A          [========]                 
B                      [==]           
C                           [========]
D               [==============]      
OVERLAP         [===]  [==] [==]      

I'm definitely open to ways to make this visualization simpler to create (and more robust) but I wanted to go ahead and get the PR opened.

Ideas:

  1. Auto-choosing the width. Not sure what a good way to do this is as the number of days encompassing all periods is definitely going to be too big for even small blocks of time.
  2. Maybe just a default width instead?
  3. (BC Break) Change overlap, diff, and friends to return subclasses with reference to the compared period to allow a simpler visualization API usable directly off of the period or collection. Something like $periodOrCollection->visualize(["width" => 25])
brendt commented 5 years ago

Just a heads up, I'll try to merge this PR this week :)

nyamsprod commented 5 years ago

@brendt @thecrypticace I've shamelessly borrowed your code to create a visualizer for League/Period... maybe you can also borrow back some of the code to improve this PR πŸ˜‰ you can see the package here https://github.com/bakame-php/period-visualizer Also I choose to make a independant package because for the casual user having a visualizer is not really that important anyway thanks @thecrypticace for the great ground work.

brendt commented 5 years ago

Embracing the power of OSS! πŸ’ͺ

brendt commented 5 years ago

Great work @thecrypticace !

brendt commented 5 years ago

Tagged as 0.4.0: https://github.com/spatie/period/releases/tag/0.4.0

thecrypticace commented 5 years ago

πŸŽ‰

brendt commented 5 years ago

@thecrypticace I'll be adding boundary types later this week. This will also have an influence on the visualization helper: a inclusive boundary is noted with [, while an exclusive boundary is noted with (. More info can be found here: https://github.com/spatie/period/issues/9

If you want, I'll let you know when the feature is done, so that you can update the visualization class. If you're not up for it, just let me know and I'll add it!

nyamsprod commented 5 years ago

@thecrypticace @brendt you should let the user of the visualizer decide how they will/want to represent the intervals see here https://github.com/bakame-php/period-visualizer#customize-the-output I've just switched the options array to a Config object.

thecrypticace commented 5 years ago

@brendt sounds good. I'll definitely take that up once that's in. @nyamsprod This is a good idea. I like it!

These additions will necessitate changes to the matrix building to denote the boundary type in the appropriate cells. (at least in a first pass β€” a second would preserve the actual objects for checking)

nyamsprod commented 5 years ago

@thecrypticace why do you want to change the matrix ? the matrix is independant of the boudaries πŸ€”

thecrypticace commented 5 years ago

Because the matrix is used to build the visualization. If you represent open or closed ends with different characters the part that creates the strings will need to know about it. There are other ways to do it too just thinking out loud.

nyamsprod commented 5 years ago

Well let the config array or object deal with that

brendt commented 5 years ago

@thecrypticace Boundaries are now supported (0.5.0), would you like to take a look on how we can improve the visualizer?

It's my opinion that a fully configurable visualizer is not within the scope of this project; it's a debug helper. Hence I think that it's perfectly fine to hard code the [] and () notation for included and excluded boundaries. But feel free to argue against me 😁