Adopt ginga 7.1's public set_hash API in the stretch adapter - #229
Open
mwcraig wants to merge 2 commits into
Open
Adopt ginga 7.1's public set_hash API in the stretch adapter#229mwcraig wants to merge 2 commits into
mwcraig wants to merge 2 commits into
Conversation
Upstream fixed ejeschke/ginga#1148 by adding a public set_hash() helper for ColorDist subclasses and a backward-compat shim in check_hash() that rescues old-contract integer hashes with a PendingDeprecationWarning. Probe for the new contract via hasattr(ColorDistBase, 'set_hash') instead of sniffing the hash dtype of a throwaway LinearDist, and add a regression test asserting the adapter never triggers ginga's deprecation shim. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_019GuBhnNvskTtCsb6Pw69BC
On ginga >= 7.1 install the normalized 0..1 curve through the public set_hash() helper added in the fix for ejeschke/ginga#1148 instead of assigning self.hash directly. The legacy integer-levels branch stays until astrowidgets requires ginga >= 7.1; released ginga (<= 7.0) has no set_hash and uses the hash directly as color indices. Also add the missing changelog entry for astropy#226. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_019GuBhnNvskTtCsb6Pw69BC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #226. The upstream issue we reported, ejeschke/ginga#1148, was fixed on ginga master (7.1.0.dev17): ginga now provides a public
set_hash(base)helper as the supported way forColorDistsubclasses to install their normalized 0..1 curve, plus a build-time backward-compat shim incheck_hash()that rescues old-contract integer hashes with aPendingDeprecationWarning.This PR updates our compatibility code to track that resolution:
_AstropyStretchDist.calc_hashnow installs the curve through the publicset_hash()on ginga >= 7.1 instead of assigningself.hashdirectly; the integer-levels branch is kept for released ginga (<= 7.0), with a TODO to drop it once we require ginga >= 7.1.hasattr(ColorDist.ColorDistBase, "set_hash")rather than sniffing the hash dtype of a throwawayLinearDist.We cannot remove the dual-contract code yet: released ginga still requires the integer-levels representation and has no
set_hash, andpyproject.tomlallowsginga>=3.4.Testing
pixi run testagainst ginga 7.0.0: 236 passed, 2 skipped (legacy branch).-W error::PendingDeprecationWarning, confirmingset_hash()is used and the compat shim never fires.Co-written with Claude Fable 5
https://claude.ai/code/session_019GuBhnNvskTtCsb6Pw69BC