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

WIP: Add extended component #2373

Closed
wants to merge 20 commits into from

Conversation

jfoster17
Copy link
Member

@jfoster17 jfoster17 commented Mar 6, 2023

Pull Request Template

Description

Here is one approach to displaying many regions (e.g. regions-as-data) in glue (ala #2351). I've implemented this for the first place I wanted it, which is as a different kind of ScatterLayer in the ImageViewer.

The pieces are basically:

  • An ExtendedComponent(Component) which is a list of Shapely geometries
  • A RegionData(Data) which is probably unnecessary (more Data types are hard to support) but for now is used to determine if a Data object should be plotted as regions or as points.
  • ScatterRegionLayerState and ScatterRegionLayerArtist classes to deal with actually configuring and plotting regions
  • Some code from GeoPandas (with LICENSE) for the actual plotting/updating of arbitrary polygons.

Major unfinished things

  • Projections/transformations don't work on regions (would need to apply transformation to all points in the region definition) -- now working for images with WCS. Defer the more general case until we work on the general Scatter viewer.
  • Regions can be displayed and selected, but cannot be used for logical operations
  • Regions are selected into subsets based on their representative point (a cheap centroid that must be inside the geometry). This might not always be the desired behavior.
    It's much less clear what the best behavior is when adding RegionData to a scatterplot. The assumption is that if you have regions, you want to plot regions on an image (perhaps color-coded by another component). If you have a table of data about regions you might want to make a scatterplot that just shows those regions (i.e. you don't have an image and just want to see the geometry) or you might want to make a scatterplot 2 components of those regions.

@jfoster17
Copy link
Member Author

@jfoster17 jfoster17 marked this pull request as draft March 6, 2023 22:36
@astrofrog
Copy link
Member

@jfoster17 this is very nice! It looks like this will need rebasing due to conflicts. Would you like me to review this yet at this point or are there things you still need to work on before a first review?

@jfoster17
Copy link
Member Author

There are still a few more things I need to fix/add before this is ready for review. I will try to keep it to the smallest amount required to be useful though, so that it doesn't get too large.

@jfoster17
Copy link
Member Author

I wanted to highlight 79e4fa5, because it opens (or increases?) the potential for malicious session files and deserves some consideration. If I'm going to represent regions as Shapely objects we need something like this to enable save/restore of sessions that bundle the data. This also solves #2330.

@astrofrog
Copy link
Member

I'd rather not use allow_pickle if at all possible - is there any way we can serialize/deserialize the Shapely objects to strings?

@astrofrog
Copy link
Member

As noted in #2430, I will be splitting out Qt-related code into a separate repository - however that repository will have the same history as the present one here so it should be easy to split the Qt-specific parts of this PR into a separate PR to glue-qt. Sorry for the disruption!

@jfoster17 jfoster17 mentioned this pull request Sep 13, 2023
@jfoster17
Copy link
Member Author

Closing in favor of #2442

@jfoster17 jfoster17 closed this Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants