Export JPG with AVM (WCS metadata)#4065
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (33.33%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #4065 +/- ##
==========================================
- Coverage 84.60% 84.51% -0.10%
==========================================
Files 202 203 +1
Lines 29402 29436 +34
==========================================
+ Hits 24876 24878 +2
- Misses 4526 4558 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@bmorris3 - just FYI I released pyavm 0.9.9 today which fixes a few bugs. I don't suspect these were affecting you here, but let me know if you have any issues with this new version. |
|
Thank you @astrofrog! |
| output file already exists. Otherwise, a message will be sent | ||
| to application snackbar instead. | ||
|
|
||
| embed_avm : bool |
There was a problem hiding this comment.
would there be any harm to having this on by default (or not even have the option to turn it off)?
There was a problem hiding this comment.
I don't think there's any harm. I think we should preserve the option to turn it off, just in case there's ever an upstream bug.
There was a problem hiding this comment.
I guess the downside is that it might be confusing for non-supported filetypes if the user passes this as true (or leaves it at the default) and we don't have checking that raises an error 🤔
|
|
||
| try: | ||
| # export PNG | ||
| tmp_filename = Path(str(Path(filename)).replace('.jpg', '.png')) |
There was a problem hiding this comment.
can this be done by writing to a buffer instead of a temporary file to disk?
There was a problem hiding this comment.
I think pyavm is built around a file on disk, see for example: https://github.com/astrofrog/pyavm/blob/9152f6809426b40647ecbaefcefceabe09236d94/pyavm/jpeg.py#L64-L66
Is that right @astrofrog?
|
Is there any reason not to allow PNG export already as other tools may be able to use them? |
Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>
pcuste1
left a comment
There was a problem hiding this comment.
Pulled this PR down locally and it is working as advertised. As noted, it hasn't been designed with rotation in mind, and it handles at bit weird once the imviz viewer is rotated. This is likely in part due to the missing alpha channel and the change in FOV caused by rotation. We'll want to look further into this in a future PR. I'm also curious how it will handle projection once we introduce the feature to update that into jdaviz. Preliminary tests also show that WCS near the poles are handled properly as well
Description
The main goal of this PR is to allow users to export a screenshot of their image viewer and load that screenshot into aladin-lite or mast-aladin. In order to overlay the screenshot on the correct sky coordinates in aladin-lite, the screenshot must contain AVM.
AVM is a metadata standard for embedding WCS in common image formats, including PNG and JPG. The python module for embedding AVM from WCS is pyavm by @astrofrog, which is a new (and extremely minimal) dependency in this PR.
This PR adds a supported image viewer screenshot filetype (
"jpg"). When selected, the Export plugin:The result is a JPG file with AVM. You can check this by calling:
See that madness? That's the WCS.
Now you can load the JPG with aladin-lite in your browser, or in an instance of mast-aladin. In either environment, right click* on the sky , select "Load a local file" > "FITS image"*, and select the
imviz-0.jpgfile (*see known issues below).To try this out in a notebook with mast-aladin, run the following in one cell:
and in a second cell:
You will see this:
jdaviz-to-aladin-export-720.mov
Known issues
To do items before merge:
Change log entry
CHANGES.rst? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rstbefore merge. If no, maintainershould add a
no-changelog-entry-neededlabel.Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
triviallabel.cache-download.ymlworkflow?