Skip to content

Conversation

@Hartorn
Copy link
Member

@Hartorn Hartorn commented Dec 12, 2023

No description provided.

@Hartorn Hartorn self-assigned this Dec 12, 2023
@Hartorn Hartorn changed the base branch from main to dataset-first December 12, 2023 15:32
@Hartorn Hartorn force-pushed the rework-enum branch 2 times, most recently from 6d46ff3 to 39a4a52 Compare December 12, 2023 16:57
Base automatically changed from dataset-first to main December 13, 2023 13:02
@Hartorn Hartorn marked this pull request as ready for review December 18, 2023 16:37
Copy link
Contributor

@rabah-khalek rabah-khalek left a comment

Choose a reason for hiding this comment

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

Great work @Hartorn, quite an improvement and I particularly like the operators extension to the names.

I left some comments for the future, particularly the direction of using https://numpy.org/doc/stable/reference/maskedarray.generic.html to define a new "marks" class that already support masks (our facial_parts) with all the native ops and validations numpy.ma has.

Comment on lines -8 to -19
class FacialPart(NDArrayOperatorsMixin):
part: np.ndarray
name: str = ""

def __add__(self, o):
return FacialPart(np.unique(np.concatenate((self.part, o.part))))

def __sub__(self, o):
return FacialPart(np.setxor1d(self.part, np.intersect1d(self.part, o.part)))

def __array__(self):
return self.part
Copy link
Contributor

Choose a reason for hiding this comment

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

Inheriting from NDArrayOperatorsMixin and having __array__ would avoid us having crop_mark

Comment on lines 18 to 23
def crop_mark(mark: np.ndarray, part: FacialPart, exclude=False):
idx = np.isin(FacialParts.entire, part)
if not exclude:
idx = ~idx
part = ~part
res = mark.copy()
res[idx, :] = np.nan
res[part.idx, :] = np.nan
return res
Copy link
Contributor

Choose a reason for hiding this comment

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

Reflecting on all this, I think https://numpy.org/doc/stable/reference/maskedarray.generic.html could be the most suited for our case.

@rabah-khalek rabah-khalek merged commit 27ae475 into main Dec 20, 2023
@rabah-khalek rabah-khalek deleted the rework-enum branch December 20, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants