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

Profile drawing and masked regions ... #3285

Closed
1 of 3 tasks
kif opened this issue Nov 24, 2020 · 3 comments · Fixed by #3369
Closed
1 of 3 tasks

Profile drawing and masked regions ... #3285

kif opened this issue Nov 24, 2020 · 3 comments · Fixed by #3369
Milestone

Comments

@kif
Copy link
Member

kif commented Nov 24, 2020

After discussion with Thomas, we agree this is a wished feature. Nevertheless taking into account the mask should be wired down at the array level and not connecting at the mask widget as it is proposed in #3281. This will simplify also the calculation of the range of the colormap with masked data (especially important with Eiger2 detectors are the dummy value is above 4 billions, 2^32-1)

This approach will require much more work at lower levels but will allow the generalization to all kind of processing very easily.

Here is a TODO list:

  • Provide the associated mask in the image object like: image.getData() one could have image.getMask()
  • Profile calculation are performed in Cython. It is the right time to refactor the code to take mask into account
  • Wire the mask in the ProfileToolBar.

The mask should be an array of same size as the image with dtype uint8 (or None if absent)
The alternative, numpy.maskedarray, has been discarded because we which to be able to manage multiple masks, either 7 overlapping masks or 254 non overlapping masks.

Sponsor: ID17

@t20100
Copy link
Member

t20100 commented Nov 25, 2020

I had a look at it, and it seems fine to integrate the mask at the low level.
It is also more versatile (support of mask loaded from a Nexus file should be simpler).

The only adaptation in PlotWidget beyond to image item seems to be that a plot items should be able to add more than one renderer item in the backend (which totally makes sense and should not be a big deal).
The most complex part can be expected in the mask authoring tools which will need to be updated.... and of course all tools willing to take the mask into account. Having the mask stored with the image item mask will make the wiring/update notification much simpler.

@t20100
Copy link
Member

t20100 commented Nov 25, 2020

If possible I would prefer not to include that in 0.14, since the idea was to make a release quickly.
What do you think?

@kif
Copy link
Member Author

kif commented Nov 25, 2020

For me it is OK. Just doing the Cython part is already half a day, having it ready for next week looks un-realistic.

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