olavolav / uniplot

Lightweight plotting to the terminal. 4x resolution via Unicode.
MIT License
343 stars 16 forks source link

Histogram x-axis range ignored #24

Closed riga closed 4 months ago

riga commented 4 months ago

Hi again,

after playing around a bit more with histograms, I noticed that the x-axis range is ignored in cases where the number of bins as well as min/max values are provided (histogram(data, bins=..., x_min=..., x_max=...)).

Here is a reproducer:

import numpy as np
from uniplot import histogram

data = np.random.uniform(0, 3, size=1000)
histogram(data, bins=4, x_min=-0.5, x_max=3.5)

# ->
┌────────────────────────────────────────────────────────────┐
│       ▐▀▀▀▀▀▀▀▀▀▀▜          ▐▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▌       │
│       ▐          ▐          ▐                      ▌       │
│       ▐          ▝▀▀▀▀▀▀▀▀▀▀▀                      ▌       │
│       ▐                                            ▌       │ 200
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │ 100
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│       ▐                                            ▌       │
│▁▁▁▁▁▁▁▐▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▌▁▁▁▁▁▁▁│ 0
└────────────────────────────────────────────────────────────┘
        0              1              2              3

The axis range is of course correct, but I would have expected that the left edge of the first bin and the right edge of the last bin landed on -0.5 and 3.5, respectively.

I could provide a PR, but I first wanted to check if the current behavior was intended.

olavolav commented 4 months ago

Hi @riga always happy for feedback!

That is correct behavior since x_min and x_max are simply forwarded to the plotting logic, i.e. they determine the view window and not – as you probably expected – the limits of the histogram. What you are looking for is called bins_min and bins_max in the code, though I have never fully implemented the logic.

I have a fix here that might help. In that version:

import numpy as np
from uniplot import histogram

data = np.random.uniform(0, 3, size=1000)
histogram(data, bins=4, bins_min=-0.5, bins_max=3.5)

yields

┌────────────────────────────────────────────────────────────┐
│       │       ▛▀▀▀▀▀▀▀▀▀▀▀▀▀▀▙▄▄▄▄▄▄▄▄▄▄▄▄▄▄               │
│       │       ▌                            ▐               │ 300
│       │       ▌                            ▐               │
│       │       ▌                            ▐               │
│       │       ▌                            ▐               │
│       │       ▌                            ▐               │
│       │       ▌                            ▐               │ 200
│▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▌                            ▐               │
│▌      │                                    ▐▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄│
│▌      │                                                   ▐│
│▌      │                                                   ▐│
│▌      │                                                   ▐│ 100
│▌      │                                                   ▐│
│▌      │                                                   ▐│
│▌      │                                                   ▐│
│▌      │                                                   ▐│
│▌▁▁▁▁▁▁│▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▐│ 0
└────────────────────────────────────────────────────────────┘
        0              1              2              3

Makes sense? If so I can merge it.

If you find the feature useful, maybe it makes sense to take this feature out of the experimental stage and fully include it in the documentation? Let me know what you think

riga commented 4 months ago

Hi @olavolav ,

that sounds very reasonable and useful indeed, and I like the separation of ranges for the view and the binning logic. Looking forward to that feature :+1:

olavolav commented 4 months ago

This is now live as release v0.12.4 🚀

Thanks again!