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

Persistence graphical tools for sklearn itf #1157

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

VincentRouvreau
Copy link
Contributor

No description provided.

@VincentRouvreau VincentRouvreau added the enhancement New feature or request label Dec 2, 2024
Copy link
Member

@mglisse mglisse left a comment

Choose a reason for hiding this comment

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

If we computed only dimensions 2 and 4, they will be labelled 0 and 1 in the legend. But we can see later about adding a way to pass the homology_dimensions (from RipsPersistence and others) to this function.

first_death_value = elt[0][1]
if not isinstance(first_death_value, (np.floating, float)):
raise TypeError("Should be a list of (birth,death)")
pers.extend([fake_dim, x] for x in elt)
Copy link
Member

Choose a reason for hiding this comment

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

It is a bit sad to convert from the efficient numpy format to the inefficient list, when what matplotlib expects is close to the original array. But that's an implementation detail, we can revisit it later if it turns out to be non-negligible.

# Array
try:
first_death_value = a[0][1]
if isinstance(first_death_value, (np.floating, float)):
Copy link
Member

Choose a reason for hiding this comment

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

I know we already had that, but I wonder if we should also accept integers here?

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

Successfully merging this pull request may close these issues.

2 participants