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

Add radius keyword to bokeh.figure.circle calls #1643

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

bhilbert4
Copy link
Collaborator

Similar to the fix for the readnoise monitor plots in #1631, this PR adds the 'circle' keyword to other instances of bokeh.figure.circle() calls in the JWQL codebase. In some cases there was no keyword indicating glyph size. In other cases, 'radius' was used, which is now deprecated.

Local testing showed everything looks ok, with the exception of the bad pixel monitor and the EDB telemetry monitor, which failed due to bokeh version mismatches. (Actually, we may need to run the monitors to see updated plots in these cases, since the plots are not produced on the fly).

@bhilbert4 bhilbert4 self-assigned this Aug 29, 2024
@pep8speaks
Copy link

pep8speaks commented Aug 29, 2024

Hello @bhilbert4, Thank you for updating !

Line 995:13: E128 continuation line under-indented for visual indent
Line 1001:41: E128 continuation line under-indented for visual indent
Line 1142:17: E128 continuation line under-indented for visual indent
Line 1150:17: E128 continuation line under-indented for visual indent

Line 228:83: E231 missing whitespace after ','

Comment last updated at 2024-09-19 01:51:49 UTC

@bhilbert4
Copy link
Collaborator Author

@mfixstsci coming back to this PR, it might be easiest to review and merge, and then check the results on the test server? The pages that are failing to load are failing because of a bokeh version mismatch between the html file and the installed version. I think we need to re-run the monitors themselves (rather than just the data retrieval/plotting functions) in order to regenerate the html file with the current version of bokeh before we'll be able to load the page.

@mfixstsci
Copy link
Collaborator

@mfixstsci coming back to this PR, it might be easiest to review and merge, and then check the results on the test server? The pages that are failing to load are failing because of a bokeh version mismatch between the html file and the installed version. I think we need to re-run the monitors themselves (rather than just the data retrieval/plotting functions) in order to regenerate the html file with the current version of bokeh before we'll be able to load the page.

Assuming the monitor will rewrite the bokeh figures, this idea should work. Worst case, we just delete the file on disk and regenerate it.

Copy link
Collaborator

@mfixstsci mfixstsci left a comment

Choose a reason for hiding this comment

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

Changes look good, thanks @bhilbert4

@bhilbert4 bhilbert4 merged commit 08d88ad into spacetelescope:develop Sep 19, 2024
11 checks passed
@bhilbert4 bhilbert4 deleted the fix-all-bokeh-circle-figs branch September 19, 2024 15:11
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