hips2fits: make Pillow an optional dependency with a clear ImportError#3619
hips2fits: make Pillow an optional dependency with a clear ImportError#3619kushasahu7 wants to merge 3 commits into
Conversation
PIL was imported unconditionally at module level, causing a hard crash on `import astroquery.hips2fits` when Pillow is not installed, even though it is only needed for jpg/png response decoding. Moved the import inside _parse_result behind a try/except ImportError, and added Pillow to the `all` extras in pyproject.toml so it is properly declared as optional. Added unit tests that verify a helpful ImportError is raised for both jpg and png formats when Pillow is absent. Closes astropy#3556
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3619 +/- ##
==========================================
- Coverage 73.16% 73.15% -0.02%
==========================================
Files 227 227
Lines 21077 21081 +4
==========================================
Hits 15422 15422
- Misses 5655 5659 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@bsipocz When you have some time, can you review this PR ? |
bsipocz
left a comment
There was a problem hiding this comment.
this needs some cleanup, but the big picture looks good.
thanks
| mock_response.status_code = 200 | ||
| mock_response.content = b'fake_image_bytes' | ||
|
|
||
| with patch.dict(sys.modules, {'PIL': None, 'PIL.Image': None}): |
There was a problem hiding this comment.
I don't think this is the correct approach, use pytest markers instead to check for the missing pillow install.
| with pytest.raises(ImportError, match="Pillow is required"): | ||
| hips2fits._parse_result(mock_response, verbose=False, format='jpg') | ||
|
|
||
| def test_png_format_requires_pillow(self): |
There was a problem hiding this comment.
These two tests are practically identical, please use pytest parametrize
| - ``Pillow`` is now an optional dependency: importing ``astroquery.hips2fits`` no | ||
| longer crashes when Pillow is not installed. A clear ``ImportError`` is raised | ||
| only when a jpg/png response is actually decoded. [#3619] |
There was a problem hiding this comment.
This needs a rewrite, pillow has been an optional dependency already.
| - ``Pillow`` is now an optional dependency: importing ``astroquery.hips2fits`` no | |
| longer crashes when Pillow is not installed. A clear ``ImportError`` is raised | |
| only when a jpg/png response is actually decoded. [#3619] | |
| - Fix the import error for not available optional dependency Pillow. A clear ``ImportError`` is raised | |
| only when a jpg/png response is actually decoded. [#3619] |
|
Thanks for the review @bsipocz! Addressed all three:
|
Summary
Fixes the remaining open item in #3556.
PILwas imported unconditionally at module level inhips2fits/core.py, causing a hardModuleNotFoundErroronimport astroquery.hips2fitswhen Pillow is not installed — even though it is only needed when decoding jpg/png responses.Changes
from PIL import Imageinside_parse_result()behind atry/except ImportErrorwith a helpful install messagePillowto thealloptional extras inpyproject.tomlRelated
boto3/botocorein MAST: Enable cloud dataset by default #3534