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

Turn off thumbnail creation for all but rate files #968

Merged

Conversation

bhilbert4
Copy link
Collaborator

Resolves #957

This PR makes changes to the preview image generation code such that thumbnail images are created only for rate files. Currently we are creating thumbnail files for all file types (uncal, rate, cal, etc). However, we only need one thumbnail for each file, since the instrument archive pages do not show separate images for each file type.

@pep8speaks
Copy link

pep8speaks commented Jul 1, 2022

Hello @bhilbert4, Thank you for updating !

Line 50:1: E402 module level import not at top of file
Line 51:1: E402 module level import not at top of file
Line 52:1: E402 module level import not at top of file

Line 54:34: E126 continuation line over-indented for hanging indent
Line 77:1: E402 module level import not at top of file
Line 78:1: E402 module level import not at top of file
Line 217:64: E226 missing whitespace around arithmetic operator
Line 982:13: E265 block comment should start with '# '
Line 1016:5: E265 block comment should start with '# '
Line 1078:17: W503 line break before binary operator
Line 1079:17: W503 line break before binary operator
Line 1080:17: W503 line break before binary operator
Line 1081:17: W503 line break before binary operator
Line 1416:5: E265 block comment should start with '# '
Line 1416:5: E303 too many blank lines (2)
Line 1417:5: E265 block comment should start with '# '
Line 1418:5: E265 block comment should start with '# '
Line 1419:5: E265 block comment should start with '# '
Line 1420:5: E265 block comment should start with '# '
Line 1421:5: E265 block comment should start with '# '
Line 1435:5: E303 too many blank lines (2)
Line 1503:9: E722 do not use bare 'except'

Comment last updated at 2022-07-22 16:53:08 UTC

@bhilbert4
Copy link
Collaborator Author

bhilbert4 commented Jul 1, 2022

  • Make sure the pages that show thumbnails for a given proposal are only looking for thumbnail files from rate images
  • Test on dev or test server
  • Dark current observations don't produce rate files. They have uncal.fits and dark.fits. Adjust code to work for one of these filetypes, just for dark observations.

@bhilbert4
Copy link
Collaborator Author

Interested to hear what @BradleySappington thinks of these changes, since he's spent a lot of time recently looking at preview/thumbnail image generation.

@BradleySappington
Copy link
Collaborator

This looks good with the understanding that the dark files will not be created until the third task is completed.

Copy link
Collaborator

@BradleySappington BradleySappington left a comment

Choose a reason for hiding this comment

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

Looks good, would hold off on merge until dark code is included.

@bhilbert4
Copy link
Collaborator Author

First test on the dev server failed due to a typo in the filename to save the thumbnails to. Typo has been fixed in the latest commit, and this is ready for re-testing on the dev server. Concentrate on program 1062, whose thumbnails I removed.

@bhilbert4 bhilbert4 changed the title [WIP]: Turn off thumbnail creation for all but rate files Turn off thumbnail creation for all but rate files Jul 22, 2022
@bhilbert4
Copy link
Collaborator Author

Tested on the test server. The code successfully made thumbnail files only for rate and dark files, as expected. I then tested and these new thumbnail images were successfully displayed on pages for individual programs.

@mfixstsci I think this is ready for review. Once this is in, I think we'll be in good shape to regenerate thumbnails on the ops server.

@bhilbert4
Copy link
Collaborator Author

Ah sorry, I forgot that @BradleySappington reviewed this earlier. Either one or both of you are welcome to check the latest updates.

@mfixstsci
Copy link
Collaborator

@BradleySappington would you like to do the honors and merge it?

@BradleySappington
Copy link
Collaborator

yessir!

@bhilbert4
Copy link
Collaborator Author

Thanks!

@BradleySappington
Copy link
Collaborator

@mfixstsci I currently lack authorization to merge

@mfixstsci
Copy link
Collaborator

mfixstsci commented Jul 22, 2022

That's okay @BradleySappington, the branch was stale so we had to merge all of our recent updates into this branch. Once the tests pass, send it.

@BradleySappington
Copy link
Collaborator

Tests passed, but merging is blocked because I'm not authorized. This is a "protected branch"

@BradleySappington
Copy link
Collaborator

Screen Shot 2022-07-22 at 1 14 32 PM

@BradleySappington BradleySappington merged commit df93bac into spacetelescope:develop Jul 22, 2022
@bhilbert4 bhilbert4 mentioned this pull request Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creation of unnecessary thumbnails
4 participants