Link pretty image from FITS file #538

wtgee · 2018-07-11T02:21:31Z

The CR2 files are linked as part of the download process. This links those generated directly from FITS files (e.g. SBIG, FLI).

Replacing parts of #511.

codecov · 2018-07-11T02:45:35Z

Codecov Report

Merging #538 into develop will decrease coverage by 0.2%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #538      +/-   ##
===========================================
- Coverage    70.55%   70.35%   -0.21%     
===========================================
  Files           64       64              
  Lines         5475     5484       +9     
  Branches       761      762       +1     
===========================================
- Hits          3863     3858       -5     
- Misses        1405     1417      +12     
- Partials       207      209       +2

Impacted Files	Coverage Δ
pocs/utils/images/__init__.py	`77.21% <ø> (+0.14%)`	⬆️
pocs/dome/astrohaven.py	`71.05% <0%> (-1.76%)`	⬇️
pocs/serial_handlers/protocol_arduinosimulator.py	`75.38% <0%> (-1.54%)`	⬇️
pocs/focuser/focuser.py	`75.4% <0%> (-1.26%)`	⬇️
pocs/camera/camera.py	`89.72% <0%> (-0.88%)`	⬇️
pocs/dome/protocol_astrohaven_simulator.py	`79.74% <0%> (-0.85%)`	⬇️
pocs/core.py	`77.16% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b3ec28...1ceef2a. Read the comment docs.

jamessynge · 2018-07-16T19:22:00Z

pocs/utils/images/__init__.py

+        pretty_path = _make_pretty_from_fits(fname, **kwargs)
+
+        # Symlink latest.jpg to the image; first remove the symlink if it already exists.
+        latest_path = '{}/images/latest.jpg'.format(os.getenv('PANDIR'))


What do you think about moving the linking for CR2 files from the cr2_to_jpg.sh script into this file?
Then we don't have two different approaches. And we don't have the primary==False check in just the CR2 case, and not the FITS case?

I would almost prefer to go the other way and bury the details of the linking inside _make_pretty_from_fits, which is almost what I did in the first place.

For the CR2 we use the convert shell command. We could split this out of the existing bash script but then would just be calling another bash script. Not a huge deal either way.

I can't see it making sense to complicate the two conversion to jpeg routines and bash scripts with linking to latest. It seems wrong to me.

So, let me be more definite and say that we should put the linking code into this routine, and share it between FITS and CR2.

wtgee · 2018-07-17T02:27:44Z

Changed it so there is a common linking. Note that this now saves over the original JPG so it is a reduced size and has a timestamp, which means that the timelapse that are created will be the same thing. Not necessarily a bad thing but FYI.

jamessynge · 2018-07-21T04:11:01Z

I'm not sure I understand the problem with overwriting that you're mentioning. Could you clarify?

…

On Mon, Jul 16, 2018, 4:27 PM Wilfred Tyler Gee ***@***.***> wrote: Changed it so there is a common linking. Note that this now saves over the original JPG so it is a reduced size and has a timestamp, which means that the timelapse that are created will be the same thing. Not necessarily a bad thing but FYI. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#538 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYmU-G5Wg4z3-adTv7l8PW4mDFewpXRks5uHUuggaJpZM4VKYwx> .

wtgee · 2018-07-21T08:42:09Z

I'm not sure I understand the problem with overwriting that you're mentioning. Could you clarify?

The JPG is extracted directly from the CR2 (it's an embedded thumbnail, i.e. not a conversion we perform). Originally we were keeping that copy alongside the FITS file but also resizing the JPG and adding the caption to the bottom and saving that as a different file. So for the CR2 it was not actually doing a symlink to the original file but instead generating a new JPG. The timelapse movies have been generated from the original JPGs, which means they are not resized and don't have a caption.

So with the above change we no longer have two JPGs but instead resize and caption the original and then do a symlink. As such the timelapse will start to appear with a caption (which will have an unreadable time since it's 3 fps) and will be slightly different size. Not a big deal either way and probably for the better to have only one JPG anyway.

wtgee · 2018-07-22T10:12:44Z

Note also that on PAN008 here in Sydney running a Raspberry Pi B 3 (not the +) it was taking too long for me to run the convert process (inside $POCS/scripts/cr2_to_jpg.sh) that resizes and captions. As such I am skipping that entirely and just writing the one jpeg alongside the fits and linking it to the latest in the images folder.

I do miss the time stamp as it's handy to know when the image changed. Luckily (:confused: ?) my tracking isn't very good right now so it's easy to tell when the image updated.

jamessynge · 2018-07-27T02:22:52Z

pocs/utils/images/__init__.py

@@ -88,9 +88,25 @@ def make_pretty_image(fname, timeout=15, **kwargs):  # pragma: no cover
        warn("File doesn't exist, can't make pretty: {}".format(fname))

    if fname.endswith('.cr2'):
-        return _make_pretty_from_cr2(fname, timeout=timeout, **kwargs)
+        pretty_path = _make_pretty_from_cr2(fname, timeout=timeout, **kwargs)
    elif fname.endswith('.fits'):


Do you want to include an else clause to handle the case of an unsupported extension?

jamessynge · 2018-07-27T02:23:41Z

pocs/utils/images/__init__.py

+        pretty_path = _make_pretty_from_fits(fname, **kwargs)
+
+    # Symlink latest.jpg to the image; first remove the symlink if it already exists.
+    if os.path.exists(pretty_path):


Perhaps add: and fname.endswith('.jpg')

jamessynge

I suggested a couple of changes, but I'll leave it to you to decide whether to make them. They can come later if appropriate.

jamessynge · 2018-08-26T14:14:27Z

I've updated to this version of software. I find that the pretty image is no longer being produced.
What testing/coverage of this portion of the code do we have?

wtgee · 2018-08-26T22:24:53Z

Is it not produced at all or just not linking? Any errors?

Link pretty image from FITS file

13f3767

wtgee requested review from a team July 11, 2018 02:21

jamessynge reviewed Jul 16, 2018

View reviewed changes

Move linking of latest jpg to make_pretty_image function.

2cba410

jamessynge reviewed Jul 27, 2018

View reviewed changes

jamessynge approved these changes Jul 27, 2018

View reviewed changes

Handling default file extensions better

1ceef2a

wtgee merged commit b689fc1 into panoptes:develop Jul 29, 2018

wtgee deleted the link-pretty-from-fits branch July 29, 2018 02:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link pretty image from FITS file #538

Link pretty image from FITS file #538

wtgee commented Jul 11, 2018

codecov bot commented Jul 11, 2018 •

edited

Loading

jamessynge Jul 16, 2018

wtgee Jul 16, 2018

jamessynge Jul 16, 2018

wtgee commented Jul 17, 2018

jamessynge commented Jul 21, 2018 via email

wtgee commented Jul 21, 2018 •

edited

Loading

wtgee commented Jul 22, 2018 •

edited

Loading

jamessynge Jul 27, 2018

jamessynge Jul 27, 2018

jamessynge left a comment

jamessynge commented Aug 26, 2018 •

edited

Loading

wtgee commented Aug 26, 2018

Link pretty image from FITS file #538

Link pretty image from FITS file #538

Conversation

wtgee commented Jul 11, 2018

codecov bot commented Jul 11, 2018 • edited Loading

Codecov Report

jamessynge Jul 16, 2018

Choose a reason for hiding this comment

wtgee Jul 16, 2018

Choose a reason for hiding this comment

jamessynge Jul 16, 2018

Choose a reason for hiding this comment

wtgee commented Jul 17, 2018

jamessynge commented Jul 21, 2018 via email

wtgee commented Jul 21, 2018 • edited Loading

wtgee commented Jul 22, 2018 • edited Loading

jamessynge Jul 27, 2018

Choose a reason for hiding this comment

jamessynge Jul 27, 2018

Choose a reason for hiding this comment

jamessynge left a comment

Choose a reason for hiding this comment

jamessynge commented Aug 26, 2018 • edited Loading

wtgee commented Aug 26, 2018

codecov bot commented Jul 11, 2018 •

edited

Loading

wtgee commented Jul 21, 2018 •

edited

Loading

wtgee commented Jul 22, 2018 •

edited

Loading

jamessynge commented Aug 26, 2018 •

edited

Loading