Change get methods to properties #76
Merged
Conversation
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
mwcraig
force-pushed
the
change-gets-to-properties
branch
from
7b4e285 to
5f31213
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR replaces parameterless getter methods with properties to more idiomatically expose image labels and catalog names, ensures None values are never returned, and updates related implementations and tests.
- Converted abstract methods
get_image_labelsandget_catalog_namesinto@property-basedimage_labelsandcatalog_namesreturning tuples - Updated
ImageViewerLogicto filter outNoneand copy docstrings from the interface - Refactored test suite to call the new properties and adjust assertions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/astro_image_display_api/interface_definition.py | Replaced get_image_labels/get_catalog_names methods with @property signatures and tuple return types |
| src/astro_image_display_api/image_viewer_logic.py | Added image_labels and catalog_names properties filtering out None keys and assigned interface docstrings |
| src/astro_image_display_api/api_test.py | Updated tests to reference the new properties and modified assertions accordingly |
Comments suppressed due to low confidence (1)
src/astro_image_display_api/api_test.py:80
- [nitpick] The helper name
_get_catalog_names_as_setstill uses a “get_” prefix even thoughcatalog_namesis now a property; consider renaming it to_catalog_names_as_setfor consistency.
def _get_catalog_names_as_set(self):
This was referenced Jul 8, 2025
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.
There are a couple of get methods,
get_image_labelsandget_catalog_names, that make sense as properties because they take no paramaters. In both cases the methods are changed to properties, and only user-provided labels and names are returned, i.e.Noneis never returned as a name or label.