Skip to content

Fix corruption of an island mask in assign_leftovers - #410

Open
AlexKurek wants to merge 5 commits into
lofar-astron:masterfrom
AlexKurek:Fix-corruption-of-an-insland-mask-in-assign_leftovers
Open

Fix corruption of an island mask in assign_leftovers#410
AlexKurek wants to merge 5 commits into
lofar-astron:masterfrom
AlexKurek:Fix-corruption-of-an-insland-mask-in-assign_leftovers

Conversation

@AlexKurek

@AlexKurek AlexKurek commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Mask updating
Replaced the incorrect reassignment (mask = (mlabels == ii)) with proper in-place pixel masking (mask[tuple(cc)] = True). (There was a commented-out attempt # mask[cc] = True indicating the original intent).

Loop termination
The return was executed too early, meaning that not all pixels could be processed.

Comment thread bdsf/functions.py
return None, mask
# Convert 'cc' to a tuple to properly index the 2D numpy array
mask[tuple(cc)] = True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You removed the return statement, which changes the logic of the function. It will now continue into the else branch of if len(belongs) == 1, which is probably not what you want. OTOH, I find the original method confusing in the sense that it either returns None or a list. I'm not sure why it doesn't simply return the unaltered labels in this case, which I think it should. @darafferty, what do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, I have restored return.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, well the question remains if it should return labels, which is provided as input and will most likely not be None, or None as it was. Like I said, I'm confused about the original code, so this might as well be a bug fix.

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.

2 participants