Skip to content

Tickets/dm 15015 #9

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

Merged
merged 13 commits into from
Nov 9, 2018
Merged

Tickets/dm 15015 #9

merged 13 commits into from
Nov 9, 2018

Conversation

wmwv
Copy link

@wmwv wmwv commented Nov 9, 2018

No description provided.

Copy link
Author

@wmwv wmwv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. One request is to more clearly explain what fastMaskDisplay does. The reader is likely to misunderstand "lowest bitplane" to mean on an overall basis; but instead, what is meant is "lowest mask bit set per pixel".

with pyplot.rc_context(dict(interactive=False)):
if isMask:
for i, p in reversed(list(enumerate(planeList))):
if colors[i + 1][3] == 0:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain for the reader of the code

  1. Why i+1 instead of i?
  2. That [3] is the alpha channel. Perhaps as simple as
alpha_channel = 3
[...]
     if colors[i + 1][alpha_channel] == 0

Could do these even higher up above so that the colors[i + 1][3] = alpha in line 320 used this same alpha_channel. It's more obvious in line 320 because of the variable name on the rhs, but for consistency could do both.

extent=extent, cmap=cmap, norm=norm)
maskArr[:] = 0

if self._fastMaskDisplay: # we only draw the lowest bitplane
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should say 'we only draw the lowest bitplane set for each pixel'.

maskArr[bitIsSet] = i + 1 accumulates for each item in planeList. The maskArr is only reset if not self._fastMaskDisplay.

"""
Initialise a matplotlib display

@param fastMaskDisplay If True, only show the first bitplane that's set
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in each pixel.

@laurenam
Copy link

laurenam commented Nov 9, 2018

I think you need to rebase your branch. The update in your commit here 70abe25 seems to have already been done on this commit (on master): 63f6f0c

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

Successfully merging this pull request may close these issues.

3 participants