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

Allow to set a custom mask interpolation method #945

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

creafz
Copy link
Collaborator

@creafz creafz commented Jul 4, 2021

This is POC to fix an issue in #850. It supports two styles for setting mask interpolation.

Option 1. Set a global value for mask interpolation through Compose:

t = A.Compose([
    A.Resize(128, 128),
], mask_interpolation=cv2.INTER_NEAREST)

Option 2. Override a value for mask interpolation for a single transform:

t = A.Compose([
    A.Resize(128, 128).set_mask_interpolation(cv2.INTER_NEAREST_EXACT),
])

@creafz creafz requested review from BloodAxe, ternaus and Dipet July 4, 2021 15:34
@creafz creafz marked this pull request as draft July 4, 2021 15:35
@creafz creafz changed the title [WIP] Allow to set a custom mask interpolation method Allow to set a custom mask interpolation method Jul 4, 2021
@creafz
Copy link
Collaborator Author

creafz commented Jul 5, 2021

Or as an alternative for Option 2, maybe it is better to an explicit argument for all DualTransform subclasses, such as

t = A.Compose([
    A.Resize(128, 128, mask_interpolation=cv2.INTER_NEAREST_EXACT)
])

What do you think?

Comment on lines +121 to +135
if mask_interpolation not in {
cv2.INTER_NEAREST,
cv2.INTER_NEAREST_EXACT,
cv2.INTER_LINEAR,
cv2.INTER_LINEAR_EXACT,
cv2.INTER_AREA,
cv2.INTER_CUBIC,
cv2.INTER_LANCZOS4,
cv2.INTER_MAX,
}:
raise ValueError(
f"Value {mask_interpolation} is not supported. "
f"Choose one of the following methods: cv2.INTER_NEAREST, cv2.INTER_NEAREST_EXACT, cv2.INTER_LINEAR, "
f"cv2.INTER_LINEAR_EXACT, cv2.INTER_AREA, cv2.INTER_CUBIC, cv2.INTER_LANCZOS4, cv2.INTER_MAX"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe better to create an Enum as class field for this flag?
With Enum error message would be much more clearly.

Comment on lines +136 to +138
for t in self.transforms:
if isinstance(t, BaseCompose) or (isinstance(t, DualTransform) and t.mask_interpolation is None):
t.set_mask_interpolation(mask_interpolation)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Strange condition, I am not sure that it will be work correctly, because mask_interpolation has no default value.

@Dipet
Copy link
Collaborator

Dipet commented Jul 5, 2021

Or as an alternative for Option 2, maybe it is better to an explicit argument for all DualTransform subclasses, such as

t = A.Compose([
    A.Resize(128, 128, mask_interpolation=cv2.INTER_NEAREST_EXACT)
])

What do you think?

I think approach better, but it is need to change __init__ method for all transforms. If we will do this, we must to add **kwargs as an argument for all transforms to have possibility to add new features simpler

@Dipet Dipet added the WIP label Jun 11, 2022
@LucaBonfiglioli
Copy link

please

@ternaus
Copy link
Collaborator

ternaus commented Mar 28, 2024

@LucaBonfiglioli Will look into this.

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

Successfully merging this pull request may close these issues.

None yet

4 participants