-
Notifications
You must be signed in to change notification settings - Fork 49
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
Pretty image title #610
Pretty image title #610
Changes from 3 commits
e2b094f
35562b3
bcd86b5
8b72264
61d4c1a
6844e4f
831b6bc
4a428d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,32 +63,31 @@ def crop_data(data, box_width=200, center=None, verbose=False): | |
return center | ||
|
||
|
||
def make_pretty_image(fname, timeout=15, **kwargs): # pragma: no cover | ||
""" Make a pretty image | ||
def make_pretty_image(fname, title=None, timeout=15, **kwargs): # pragma: no cover | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about adding an issue to add testing of this? It doesn't strike me as that hard to test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #608 also contains an item about adding testing for images. We currently don't test more of our images. |
||
"""Make a pretty image. | ||
|
||
This will create a jpg file from either a CR2 (Canon) or FITS file. | ||
|
||
Notes: | ||
See `$POCS/scripts/cr2_to_jpg.sh` for CR2 process | ||
See `/scripts/cr2_to_jpg.sh` for CR2 process. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you remove $POCS? Assuming well founded, why not also remove the leading slash? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why I did this. Will fix. |
||
|
||
Arguments: | ||
fname {str} -- Name of image file, may be either .fits or .cr2 | ||
**kwargs {dict} -- Additional arguments to be passed to external script | ||
|
||
Keyword Arguments: | ||
timeout {number} -- Process timeout (default: {15}) | ||
title (None|str, optional): Title to be placed on image, default None. | ||
timeout (int, optional): Timeout for conversion, default 15 seconds. | ||
**kwargs {dict} -- Additional arguments to be passed to external script. | ||
|
||
Returns: | ||
str -- Filename of image that was created | ||
str -- Filename of image that was created. | ||
|
||
""" | ||
assert os.path.exists(fname),\ | ||
warn("File doesn't exist, can't make pretty: {}".format(fname)) | ||
|
||
if fname.endswith('.cr2'): | ||
pretty_path = _make_pretty_from_cr2(fname, timeout=timeout, **kwargs) | ||
pretty_path = _make_pretty_from_cr2(fname, title=title, timeout=timeout, **kwargs) | ||
elif fname.endswith('.fits'): | ||
pretty_path = _make_pretty_from_fits(fname, **kwargs) | ||
pretty_path = _make_pretty_from_fits(fname, title=title, **kwargs) | ||
else: | ||
warn("File must be a Canon CR2 or FITS file.") | ||
return None | ||
|
@@ -110,25 +109,30 @@ def make_pretty_image(fname, timeout=15, **kwargs): # pragma: no cover | |
return None | ||
|
||
|
||
def _make_pretty_from_fits( | ||
fname=None, figsize=(10, 10 / 1.325), dpi=150, alpha=0.2, number=7, **kwargs): | ||
def _make_pretty_from_fits(fname=None, | ||
title=None, | ||
figsize=(10, 10 / 1.325), | ||
dpi=150, | ||
alpha=0.2, | ||
number_ticks=7, | ||
clip_percent=99.9, | ||
**kwargs): | ||
|
||
with open_fits(fname) as hdu: | ||
header = hdu[0].header | ||
data = hdu[0].data | ||
data = focus_utils.mask_saturated(data) | ||
wcs = WCS(header) | ||
|
||
title = kwargs.get('title', header.get('FIELD', 'Unknown')) | ||
exp_time = header.get('EXPTIME', 'Unknown') | ||
|
||
filter_type = header.get('FILTER', 'Unknown filter') | ||
date_time = header.get('DATE-OBS', current_time(pretty=True)).replace('T', ' ', 1) | ||
if not title: | ||
field = header.get('FIELD', 'Unknown') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps 'Unknown field' and 'Unknown duration'? |
||
exp_time = header.get('EXPTIME', 'Unknown') | ||
filter_type = header.get('FILTER', 'Unknown filter') | ||
date_time = header.get('DATE-OBS', current_time(pretty=True)).replace('T', ' ', 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could read the file time as a source of a date. |
||
|
||
percent_value = kwargs.get('normalize_clip_percent', 99.9) | ||
title = '{} ({}s {}) {}'.format(field, exp_time, filter_type, date_time) | ||
|
||
title = '{} ({}s {}) {}'.format(title, exp_time, filter_type, date_time) | ||
norm = ImageNormalize(interval=PercentileInterval(percent_value), stretch=LogStretch()) | ||
norm = ImageNormalize(interval=PercentileInterval(clip_percent), stretch=LogStretch()) | ||
|
||
fig = plt.figure(figsize=figsize, dpi=dpi) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A reminder about my request that we have images (plots) that can display on both white and black backgrounds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would normally happen with the |
||
|
@@ -140,7 +144,7 @@ def _make_pretty_from_fits( | |
ra_axis.set_axislabel('Right Ascension') | ||
ra_axis.set_major_formatter('hh:mm') | ||
ra_axis.set_ticks( | ||
number=number, | ||
number=number_ticks, | ||
color='white', | ||
exclude_overlapping=True | ||
) | ||
|
@@ -149,7 +153,7 @@ def _make_pretty_from_fits( | |
dec_axis.set_axislabel('Declination') | ||
dec_axis.set_major_formatter('dd:mm') | ||
dec_axis.set_ticks( | ||
number=number, | ||
number=number_ticks, | ||
color='white', | ||
exclude_overlapping=True | ||
) | ||
|
@@ -171,16 +175,14 @@ def _make_pretty_from_fits( | |
return new_filename | ||
|
||
|
||
def _make_pretty_from_cr2(fname, timeout=15, **kwargs): # pragma: no cover | ||
def _make_pretty_from_cr2(fname, title=None, timeout=15, **kwargs): # pragma: no cover | ||
verbose = kwargs.get('verbose', False) | ||
|
||
title = '{} {}'.format(kwargs.get('title', ''), current_time().isot) | ||
|
||
solve_field = "{}/scripts/cr2_to_jpg.sh".format(os.getenv('POCS')) | ||
cmd = [solve_field, fname, title] | ||
script_name = os.path.join(os.getenv('POCS'), 'scripts', 'cr2_to_jpg.sh') | ||
cmd = [script_name, fname] | ||
|
||
if kwargs.get('primary', False): | ||
cmd.append('link') | ||
if title: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No default title? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, see PR description. |
||
cmd.append(title) | ||
|
||
if verbose: | ||
print(cmd) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, this implies to me that we should have two ways of tracking sequence id: the string needed for file paths and another for presentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
Observation
object itself should have a__str__
method that is better, but that wouldn't help much here as this is what is being passed around for the eventual FITS headers. But yes, fundamentally I agree. We pull thisreplace
a lot.