Skip to content
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

Absolute limits and MinimumRange not always honored #1812

Closed
mghie opened this issue Oct 27, 2021 · 6 comments · Fixed by #1819
Closed

Absolute limits and MinimumRange not always honored #1812

mghie opened this issue Oct 27, 2021 · 6 comments · Fixed by #1819

Comments

@mghie
Copy link
Contributor

mghie commented Oct 27, 2021

Steps to reproduce

Create an empty model with a range-constrained axis but no data, like so:

var pm = new PlotModel();
pm.Axes.Add(new LinearAxis
{
    Position = AxisPosition.Bottom,
    AbsoluteMinimum = -200.0,
    AbsoluteMaximum = -100.0,
    MinimumRange = 10.0,
});
plot1.Model = pm;

I used the WindowsFormsDemo but many of the examples should be usable as well.

Platform: OxyPlot.WinForms on Windows 10 64 bit
.NET version: 4.8

Expected behaviour

I would expect the axis to honour the absolute limits and the minimum range, so any of these visible ranges would be fine:

  • [-200.0, -100.0]
  • [-200.0, -190.0]
  • [-155.0, -145.0]
  • [-110.0, -100.0]

Actual behaviour

Instead I get the range [-100.0, -99.0] which violates both the AbsoluteMaximum and the MinimumRange properties:

oxyplot_wrong_range

Cause and proposed fix

The problem is the following code in Axis.cs:

protected virtual void CoerceActualMaxMin()
{
    // Coerce actual minimum
    if (double.IsNaN(this.ActualMinimum) || double.IsInfinity(this.ActualMinimum))
    {
        this.ActualMinimum = 0;
    }

    // Coerce actual maximum
    if (double.IsNaN(this.ActualMaximum) || double.IsInfinity(this.ActualMaximum))
    {
        this.ActualMaximum = 100;
    }

which uses values outside the allowed range. A possible fix would be to change this to:

        this.ActualMinimum = Math.Max(0, this.AbsoluteMinimum);

and

        this.ActualMaximum = Math.Min(100, this.AbsoluteMaximum);

respectively. With these changes I get the initial display range [-110.0, -100.0] which is valid.

@VisualMelon
Copy link
Contributor

VisualMelon commented Oct 30, 2021

Thanks for reporting this. Your suggested solution sounds reasonable to me. Feel free to open a PR making the change if you have time, otherwise I'll sort one out at some point. (may be worth checking the absolutes aren't NaN; not sure if that's supported)

We should consider some tests for this, since it's not something that will occur regularly if there are data on the plot.

@mghie
Copy link
Contributor Author

mghie commented Nov 2, 2021

Thanks for the reply. I have cloned the repository and will add a few tests as well. It might take me a few days though.

What would in your opinion be the best range for the axis after Reset(), given the absolute limits and the 4 possible ways to scale the axis in my example?

@VisualMelon
Copy link
Contributor

@mghieI think in this unusual scenario - where there is no data-range and the absolute min/max is non-NaN and finite - then I think assuming absolute range is probably the most sensible thing to do, but I should probably mess around with a few options myself and get back to you.

@mghie
Copy link
Contributor Author

mghie commented Nov 3, 2021

To me it's not at all unusual, but of course it depends on your use case.

I use it for visualization of the last 5 minutes of measured values in testing equipment. The time axis is therefore constrained to [-300.0, 0.0] seconds. There will never be any other time value in those plots, and the user shouldn't be able to zoom or pan outside of this time range. If the equipment is not connected there is no data to be shown.

My preference would be to have the full absolute range visible in this case.

@VisualMelon
Copy link
Contributor

@mghie for your use case, you should probably be setting the Minimum and Maximum in conjunction with the absolute limits. These will be preserved when the user zooms/pans, and are the values used when the axes are reset (when non-NaN).

Your original suggestion certainly improves matters, and would be a welcome change. It's still possible for the series to be 'upside down' in edge cases. That's a more general concern, and isn't necessarily in the scope of a PR to address this. The reality is that when the axis has nothing to go on, it's never going to give everyone what they expect.

@mghie
Copy link
Contributor Author

mghie commented Nov 12, 2021

If I understand correctly this should also close #21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants