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

NaN Values in PolarPlots #409

Open
SamKry opened this issue Jul 10, 2024 · 7 comments
Open

NaN Values in PolarPlots #409

SamKry opened this issue Jul 10, 2024 · 7 comments

Comments

@SamKry
Copy link

SamKry commented Jul 10, 2024

Description
There is a bug in the PolarPlots where NaN values are plotted incorrectly.

Steps to Reproduce
Specific steps to reproduce the behavior:

  1. Set a value of the data points to Double.NaN
  2. Use the DefaultPolarItemRenderer and plot the data
  3. Make sure strokes are visible
  4. See the NaN value pushed the line to the top left corner (See Screenshot)

Expected behavior
The NaN point should be skiped (linke Matplotlib for Python does).

Actual behavior
The NaN point is pushed to the top left corner because translateToJava2D(...) returns (0.0, 0.0) as the coordinate.

Screenshots

image

As an example, this is how NaN values are currently handled in XYPlots:

image

Java version
jdk-17.0.6.10

Additional context

The current implementation uses a poly which automatically connects the points. Since we want to skip points, we have to use the normal graphics object and do the drawing like in the XYPlots.

NOTE

I will create a PR with a fix for that issue.

@trashgod
Copy link
Contributor

I am wary of silently skipping values. Might this feature see wider acceptance as a mutable property?

@SamKry
Copy link
Author

SamKry commented Jul 23, 2024

I've looked into the code of XYPlots and pretty much implemented the renderer for PolarPlots the same way (see PR). A mutable property would of course be possible so we can control the behavior and also be backwards compatible.

But the current handling of NaN values is obviously wrong. They don't belong to the top left corner of a plot.

So there is still the question where NaN values should be displayed. In the center (0|0) would make more sense than on top-left.

Let me know what you think.

I could update the PR accordingly.

Have a nice day ☀️

@trashgod
Copy link
Contributor

Thank you for responding. In either XYPlot or PolarPlot, I see no a priori way to distinguish between erroneous data values and intentionally NaN values. In XYPlot, a gap remains as a visible indicator of possible error; in PolarPlot, skipped values may obscure possible error such as erroneous conversion. Although I have never encountered the need for the proposed feature, I would argue against making it the default.

@SamKry
Copy link
Author

SamKry commented Jul 23, 2024

I am sorry, i didn't meant skipping them. There is also a gap like in the XYPlots.

Before:

image

Same data points with the updated PolarPlot Renderer:

image

The NaN value creates a gap in the line. Is this what you were talking about?

@trashgod
Copy link
Contributor

Yes, I believe so. Your images make it clear: in the first, I can see the NaN and work out how to handle it; in the second, I don't see anything unusual. In effect, information has been hidden.

I can't argue that the proposed feature would never be useful. As others may rely on the existing behavior, I would caution against making it a new default.

For reference, a similar issue arose in StandardXYItemRenderer, discussed here +ca. v.1.0.2.

Of course, I defer to the owner.

@SamKry
Copy link
Author

SamKry commented Jul 31, 2024

Thank you for pointing out this discussion.
I have checked out the StandardXYItemRenderer and as far as I see NaN values are handled the same as i suggested. They are not shown in the chart and there is no connecting line.

XYPlot with StandardXYItemRenderer :

image

Demo Code:
 public static void main(String args[]) throws Exception {
        JFrame frame = new JFrame();
        JPanel panel = new JPanel();
        panel.setLayout(new BorderLayout());

        XYSeries s1 = new XYSeries("Series 1", true, false);

        s1.add(2, 3.4);
        s1.add(3, 5.4);
        s1.add(4, Double.NaN);
        s1.add(5, 1.1);
        s1.add(6, Double.NaN);
        s1.add(7, Double.NaN);
        s1.add(8, 6.4);
        s1.add(9, 6.5);
        DefaultTableXYDataset dataset = new DefaultTableXYDataset();
        dataset.addSeries(s1);
        XYDatasetTableModel tablemodel = new XYDatasetTableModel();

        tablemodel.setModel(dataset);

        JTable dataTable = new JTable(tablemodel);
        JScrollPane scroll = new JScrollPane(dataTable);
        scroll.setPreferredSize(new Dimension(600, 150));

        JFreeChart chart = ChartFactory.createXYLineChart(
                "XY Series Demo",
                "X", "Y", dataset, PlotOrientation.VERTICAL,
                true,
                true,
                false);

        StandardXYItemRenderer renderer = new StandardXYItemRenderer();
        renderer.setDrawSeriesLineAsPath(true);
        // enable shapes
        renderer.setBaseShapesVisible(true);

        ((XYPlot) chart.getPlot()).setRenderer(renderer);

        ChartPanel chartPanel = new ChartPanel(chart);

        panel.add(chartPanel, BorderLayout.CENTER);
        panel.add(scroll, BorderLayout.SOUTH);

        frame.setContentPane(panel);
        frame.setSize(600, 500);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.show();
    }

I understand that there might be users who got used to the way it was before where NaN values were plot in the top left corner.

Since i got used to the way NaN values are handled in XYPlots, I think it would make sense to have the same handling in all plots for consistency.

I could implement the new updated PolarPlot renderer so that only when calling a function like renderer.hideNaNValues(true) is called the new handling is used. So we still have the old handling with the NaN being at topleft as default but we can also switch to the new handling.

What do you think?

@trashgod
Copy link
Contributor

Yes, a mutable propertye.g. your hideNaNValues set to false by default—would be less disruptive.

To be honest, I would be unlikely to use the feature. I typically handle anomalous values upstream of the data model, and then only having verified the desired functionality. For example, rather than compromise data integrity, I might propose an adjacent visual control to show or hide the NaN values.

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

No branches or pull requests

2 participants