Skip to content

hips2fits: make Pillow an optional dependency with a clear ImportError#3619

Open
kushasahu7 wants to merge 3 commits into
astropy:mainfrom
kushasahu7:fix-hips2fits-pillow-optional-dep
Open

hips2fits: make Pillow an optional dependency with a clear ImportError#3619
kushasahu7 wants to merge 3 commits into
astropy:mainfrom
kushasahu7:fix-hips2fits-pillow-optional-dep

Conversation

@kushasahu7

Copy link
Copy Markdown

Summary

Fixes the remaining open item in #3556.

PIL was imported unconditionally at module level in hips2fits/core.py, causing a hard ModuleNotFoundError on import astroquery.hips2fits when Pillow is not installed — even though it is only needed when decoding jpg/png responses.

Changes

  • Moved from PIL import Image inside _parse_result() behind a try/except ImportError with a helpful install message
  • Added Pillow to the all optional extras in pyproject.toml
  • Added two unit tests covering jpg and png formats when Pillow is absent

Related

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

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.15%. Comparing base (524fc19) to head (4303d21).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
astroquery/hips2fits/core.py 0.00% 4 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kushasahu7

Copy link
Copy Markdown
Author

@bsipocz When you have some time, can you review this PR ?

@ManonMarchand ManonMarchand left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you

@bsipocz bsipocz added this to the 0.4.12 milestone Jul 18, 2026

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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}):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These two tests are practically identical, please use pytest parametrize

Comment thread CHANGES.rst Outdated
Comment on lines +109 to +111
- ``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]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs a rewrite, pillow has been an optional dependency already.

Suggested change
- ``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]

@kushasahu7

Copy link
Copy Markdown
Author

Thanks for the review @bsipocz! Addressed all three:

  • Dropped the sys.modules patching in favour of a HAS_PILLOW + @pytest.mark.skipif marker.
  • Merged the jpg/png tests into a single @pytest.mark.parametrize.
  • Reworded the changelog per your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: better handle optional dependencies for testing

3 participants