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

Link pretty image from FITS file #538

Merged
merged 3 commits into from
Jul 29, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion pocs/utils/images/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,20 @@ def make_pretty_image(fname, timeout=15, **kwargs): # pragma: no cover
if fname.endswith('.cr2'):
return _make_pretty_from_cr2(fname, timeout=timeout, **kwargs)
elif fname.endswith('.fits'):
Copy link
Contributor

Choose a reason for hiding this comment

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

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

return _make_pretty_from_fits(fname, **kwargs)
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'))
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

try:
os.remove(latest_path)
except FileNotFoundError:
pass
try:
os.symlink(pretty_path, latest_path)
except Exception:
warn("Can't link latest image")

return pretty_path


def _make_pretty_from_fits(
Expand Down