Skip to content

Conversation

@mwcraig
Copy link
Member

@mwcraig mwcraig commented May 6, 2025

The main reason for adding this -- and I don't really care if it stays in this package long-term -- is that it helps expose some of the unstated assumptions in the API (like interactions between the states of different settings, see #12) and provides, in the test file, an example of how downstream classes would demonstrate API compatibility.

@mwcraig mwcraig requested a review from Copilot May 6, 2025 21:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a dummy viewer implementation to serve as an example and exercise the API assumptions detailed in issue #12. Key changes include:

  • Importing and using the dummy viewer in tests
  • Adding a new test function to verify the dummy viewer instance
  • Creating a test case class that inherits from the widget API test suite

)


def test_instance():
Copy link

Copilot AI May 6, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a docstring to the test_instance function to clarify its purpose for future maintainers.

Suggested change
def test_instance():
def test_instance():
"""
Test that an instance of ImageViewer conforms to the ImageViewerInterface.
"""

Copilot uses AI. Check for mistakes.
assert isinstance(image, ImageViewerInterface)


class TestDummyViewer(ImageWidgetAPITest):
Copy link

Copilot AI May 6, 2025

Choose a reason for hiding this comment

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

[nitpick] Adding a brief docstring for TestDummyViewer could help clarify its role and intended usage, improving future maintainability.

Suggested change
class TestDummyViewer(ImageWidgetAPITest):
class TestDummyViewer(ImageWidgetAPITest):
"""
A test implementation of ImageWidgetAPITest that uses the ImageViewer
class as the widget under test.
"""

Copilot uses AI. Check for mistakes.
@mwcraig mwcraig force-pushed the add-minimal-dummy-viewer branch from cb491d5 to 1d80d7e Compare May 14, 2025 22:07
@mwcraig mwcraig requested a review from Copilot May 14, 2025 22:08
@mwcraig mwcraig changed the title Add a dummy vieweras a minimal implementation example Add a dummy viewer as a minimal implementation example May 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a dummy viewer as a minimal implementation example to expose unstated API assumptions and provide downstream API compatibility tests.

  • Adds a new test file (test_dummy_viewer.py) that verifies ImageViewer implements ImageViewerInterface and conforms to the widget API.
  • Updates tests/test_astro_image_display_api.py by removing an extraneous line to clean up the file.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/test_dummy_viewer.py Adds tests to verify implementation and API usage
tests/test_astro_image_display_api.py Removes a stray line to tidy up the test file

@mwcraig mwcraig merged commit ef02904 into astropy:main May 14, 2025
1 check passed
@mwcraig mwcraig deleted the add-minimal-dummy-viewer branch May 14, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant