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

TST: devdeps broken because _mouse_click_cb creates canvas object with coord being None #154

Closed
pllim opened this issue Nov 4, 2022 · 4 comments · Fixed by #155
Closed
Labels
bug Something isn't working

Comments

@pllim
Copy link
Member

pllim commented Nov 4, 2022

@ejeschke , do you know what happened here? This is happening with dev version of Ginga (but not the released version) and I cannot keep track of the massive refactoring that you recently did, so any advice would be greatly appreciated. Thanks!

Example log: https://github.com/astropy/astrowidgets/actions/runs/3391388780/jobs/5636513716

Minimal reproducible code:

import numpy as np
from astropy.table import Table
from astrowidgets.core import ImageWidget

# The widget
npix_side = 200
image = ImageWidget(image_width=npix_side, image_height=npix_side)

# The markers
x = np.array([20, 30, 40])
y = np.array([40, 80, 100])
input_markers = Table(data=[x, y], names=['x', 'y'])

# "Interactive" marking
image._is_marking = True
for data_x, data_y in input_markers:
    image._mouse_click_cb(image._viewer, None, data_x, data_y)

c_mark = image._viewer.canvas.get_object_by_tag('interactive-markers')
circ = c_mark.objects[0]
assert circ.coord == 'data'  # This will fail because circ.coord is None

def _mouse_click_cb(self, viewer, event, data_x, data_y):

@pllim pllim added the bug Something isn't working label Nov 4, 2022
@ejeschke
Copy link
Contributor

ejeschke commented Nov 5, 2022

I think that is due to a very recent change in CanvasObject where a canvas object's coord attribute used to default to 'data' but now defaults to None. But None will assign a data mapper when the object is added to a canvas--so the result is still the same. The reason for change was to allow the user to set a coordinate mapper for the canvas and have objects inherit that if they don't assign their own.

You can safely change that assertion to:

assert circ.coord in (None, 'data')

@ejeschke
Copy link
Contributor

ejeschke commented Nov 5, 2022

Sorry for the confusion!

@ejeschke
Copy link
Contributor

ejeschke commented Nov 5, 2022

You can also assign coord='data' when creating the Circle, if you prefer, and keep the assert as it was. That is probably preferable, because you are being explicit about what mapping is being used to draw the object when you create it.

@pllim
Copy link
Member Author

pllim commented Nov 7, 2022

Thanks, @ejeschke !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants