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

[graph] labels: add tick symbol, int precision, right margin #1931

Merged
merged 4 commits into from
Jun 29, 2023

Conversation

midichef
Copy link
Contributor

One bugfix, and a few improvements to the graph labels.

First is a bugfix to an earlier commit of mine, fixing an error during drawing.

Next, if the graph is graphing integer data, the labels are currently formatted as ints. When zoomed in, seeing axis labels like "1 1 1 1" is not so informative, so this change formats them as floats: "1.0 1.2 1.4 1.6". This only happens when zoomed in, not at the default zoomlevel or when zoomed out.

I also added tick symbols to the x-axis, to clarify exactly which column the label applies to. The symbol defaults to (Box Drawing Light Up). It's particularly helpful when graphing with dates on the x-axis, since they are wide. It's also helpful at the right edge of the graph, where labels may shift to avoid overrunning the right edge of the window. When that happens, I've changed the behavior so the rightmost label moves entirely to the left side of its tick symbol.

And the last change is to make the right margin expand dynamically, to be as large as necessary to fit the graph legend. This keeps the graph legend from covering up any plotted data points. But I capped the right margin so it takes up at most 1/4th of the window size.

@saulpw
Copy link
Owner

saulpw commented Jun 27, 2023

Thanks for pulling this together, @midichef. About this:

When zoomed in, seeing axis labels like "1 1 1 1" is not so informative

I agree with this in principle. Special-casing this for int columns makes me uneasy though. How many decimal places should be shown? If we show a "reasonable" number like 2, then for high zoom we'll just see 1.03 1.03 1.03 1.03 which is also not useful. But figuring out the right format string would be overly complicated and maybe not even possible for some users or use cases. So this is why I punted and just used the type/format string of the column itself. Particularly for dates.

If we're going the heuristic route for ints, though, can we make a function like isNumeric (maybe vd.isIntegerCol(col) and vd.isNumeric could become vd.isNumericCol(col)) that detects integral column types? For the moment it can just return col.type is int but this would allow it to be overridden if necessary. More importantly it would give us something to search for when looking for these cases.

@anjakefala anjakefala merged commit 7120da5 into saulpw:develop Jun 29, 2023
@anjakefala
Copy link
Collaborator

Tests are failing due to problems with debian. This PR is passing fine.

@midichef midichef deleted the graph_ui branch July 16, 2023 06:35
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

Successfully merging this pull request may close these issues.

3 participants