-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support dynamic components #2446
Merged
astrofrog
merged 4 commits into
glue-viz:main
from
jfoster17:support-dynamic-components
Oct 23, 2023
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e164546
Support updating layer artist limits when data changes
jfoster17 311f8d5
Make Data.update_components tell which components are changed and test
jfoster17 40a9574
Add a test for a LayerArtist responding to updated components
jfoster17 4c2c07a
Use a generic _on_components_changed method
jfoster17 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
""" | ||
Test that data.update_components() sends a NumericalDataChangedMessage | ||
that conveys which components have been changed. | ||
""" | ||
from glue.core.data import Data | ||
from glue.core.hub import HubListener | ||
from glue.core.data_collection import DataCollection | ||
from glue.core.message import NumericalDataChangedMessage | ||
|
||
import numpy as np | ||
from numpy.testing import assert_array_equal | ||
|
||
|
||
def test_message_carries_components(): | ||
|
||
test_data = Data(x=np.array([1, 2, 3, 4, 5]), y=np.array([1, 2, 3, 4, 5]), label='test_data') | ||
data_collection = DataCollection([test_data]) | ||
|
||
class CustomListener(HubListener): | ||
|
||
def __init__(self, hub): | ||
self.received = 0 | ||
self.components_changed = None | ||
hub.subscribe(self, NumericalDataChangedMessage, | ||
handler=self.receive_message) | ||
|
||
def receive_message(self, message): | ||
self.received += 1 | ||
try: | ||
self.components_changed = message.components_changed | ||
except AttributeError: | ||
self.components_changed = None | ||
|
||
listener = CustomListener(data_collection.hub) | ||
assert listener.received == 0 | ||
assert listener.components_changed is None | ||
|
||
cid_to_change = test_data.id['x'] | ||
new_data = [5, 2, 6, 7, 10] | ||
test_data.update_components({cid_to_change: new_data}) | ||
|
||
assert listener.received == 1 | ||
assert cid_to_change in listener.components_changed | ||
|
||
assert_array_equal(test_data['x'], new_data) |
This file contains 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
This file contains 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
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 was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give a real life example of what this method would do? I wonder if this should be a more general method such as
on_component_change
or something similar that could be more versatile?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure -- in glue genes we have a SyncComponent which is a component that is added to a dataset when a subset is created and is updated as that subset is updated. For instance, we can calculate gene expression levels in a spatial dataset for a given subset of genes; as we change the subset of genes, we update the values in the SyncComponent on the spatial dataset, but we need to prompt the cmap_lim_helpers so that they display the full range of values in the new component.
Thus, a Viewer can add something like this:
And the user experience is nice because changing the subset automatically scales the color to show the range of values.
This could definitely be called something more general; I just had the one example and I thought a more specific name would be more readable in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the main 'hook' that gets called could be called with a more general name, and in your case it could then call from there
update_component_limits
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sounds good. I've updated to this approach.