Skip to content
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

Implement support for linking BaseCartersianData instances #2326

Closed
astrofrog opened this issue Oct 4, 2022 · 6 comments
Closed

Implement support for linking BaseCartersianData instances #2326

astrofrog opened this issue Oct 4, 2022 · 6 comments

Comments

@astrofrog
Copy link
Member

Linking doesn't currently work properly with BaseCartesianData instances because we don't store any e.g. Component objects on the data - as we need to think of the data API as being analogous to a read-only API (so that it works properly with e.g. web data objects and so on). We should add support for linking BaseCartesianData objects.

@astrofrog
Copy link
Member Author

One of the main considerations here is that I think in the generic case we don't want to be adding/removing components from datasets based on links (see http://docs.glueviz.org/en/stable/developer_guide/linking.html for some examples). We can probably retain the API of Data[<linked component>] but internally not used components stored on the data class but query the link manager on-the-fly (which could pre-store the same information that we were storing on each dataset previously). I think this part shouldn't be too difficult though I think we will probably need to make sure we don't break API on the Data class.

Another thing to think about is how we will deal with:

https://github.com/glue-viz/glue/blob/main/glue/core/fixed_resolution_buffer.py#L51

in the fixed resolution buffer code - I think here again we will need to have a way to query the link manager for the transformation function from one component ID to another (which is maybe what we need above too).

So perhaps the first place to start is to determine whether we can add something to the link manager that will find the transformation from one component ID to another and/or transform values.

@astrofrog
Copy link
Member Author

astrofrog commented Oct 5, 2022

I think what we might want to do is basically modify:

for data in data_collection:
links = discover_links(data, self._links | self._inverse_links)
comps = {}
for cid, link in links.items():
d = DerivedComponent(data, link)
comps[cid] = d
data._set_externally_derivable_components(comps)

so that it stores it in an internal object rather than adding components to the dataset (at least it would only add derived components for the Data class). Then the BaseCartesianData class could have code that queries for the links on-the-fly.

@astrofrog
Copy link
Member Author

In fact thinking about this more I think we can actually just store the externally derived components inside the BaseCartesianData objects and just not expose the components directly, similar to what we do for world coordinates.

@astrofrog
Copy link
Member Author

Done in #2328

@pllim
Copy link
Contributor

pllim commented Oct 24, 2022

Thanks! What should Jdaviz expect from this? Would you open a POC PR downstream in Jdaviz with this new functionality?

@astrofrog
Copy link
Member Author

Yes I will show how to use this shortly

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

No branches or pull requests

2 participants