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

Fix #1045. Ensure line plot displays integer-valued data items in log display. #1046

Merged
merged 1 commit into from
May 16, 2024

Conversation

cmeyer
Copy link
Collaborator

@cmeyer cmeyer commented May 14, 2024

No description provided.

@ndellby
Copy link

ndellby commented May 15, 2024 via email

@cmeyer
Copy link
Collaborator Author

cmeyer commented May 15, 2024

Why do we care about integer data? (email by Outlook, if I can find it)

Data items can contain numpy arrays of any data type, including integer. The line plot was not displaying and showing a stack trace.

Copy link
Contributor

@Tiomat85 Tiomat85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix makes sense, forcing it to float if it isn't.
Question on your unit test though, you are asserting the min is <0.0, yet your lowest data point is 1.

@cmeyer
Copy link
Collaborator Author

cmeyer commented May 16, 2024

Fix makes sense, forcing it to float if it isn't. Question on your unit test though, you are asserting the min is <0.0, yet your lowest data point is 1.

I added some explanation to the code (force pushed). Rough answer: the returned values are log10 axis values, so will be slightly broader range than the original data and log10. i.e. 1,2,3 data values --> 0.98 -> 4 axis limits --> log10(0.98) -> log10(4). log10(0.98) < 0.0; log10(4) > 0.0. These are only very rough sanity checks; the test is really just to call the calculate function in the first place. The actual values for various scenarios are checked in different unit tests.

@cmeyer cmeyer merged commit 83376a2 into nion-software:master May 16, 2024
13 checks passed
@cmeyer cmeyer deleted the fix-1045 branch May 16, 2024 15:33
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