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.

Sign up for GitHub

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

Jump to bottom

Create more Mapper tests, and fix (#227) #228

Merged
merged 34 commits into from
Feb 7, 2020
Merged

Create more Mapper tests, and fix (#227) #228

merged 34 commits into from
Feb 7, 2020

Conversation

wreise
Copy link
Collaborator

@wreise wreise commented Jan 24, 2020 •
edited by ulupo
Loading

Reference Issues/PRs

Fixes tests for mapper [(#177), (#178), (#182), (#241)], and fixes (#227) and (#241).

wreise and others added 18 commits January 24, 2020 15:44
@wreise
Cover tests: fit_transform and fit+transform give the same thing
942556f
@wreise
Exclude edge cases
be11079
@wreise
Failing test restrictions
3c52b86
@wreise
Raise test coverage for OneDCover and CubicalCover
b52343e
@wreise
New tests for mapper cover
3192040
@wreise
Test for SimpleGap- somehow fails, even though it works in a notebook…
6b18b2c 
… example
@wreise
Correct the two-dimensional tensor test
69b4e72
@wreise
Set the number of clusters for singleton datasets, add more constrain…
35b2257 
…ts to generation of multinomialdistributions with hypothesis
@wreise
Test the list feature union
421c608
@wreise
Correct the list feature union test, add the single point classificat…
dc76409 
…ion for the firsthistogramgap
@wreise
Tests for max_frac of clusters and clustering from precomputed distances
1249d3e
@wreise
Explicit tests for list feature union, where transformers are None
5b87b17
@wreise
Tests for mappervisualization
85243ad
@wreise
Change the default valure of the color variable and the return of the…
d414b3a 
… interactive graph
Merge branch 'master' into mapper/fix_balanced_cover
875125f
Fix (giotto-ai#227)
6338d9a 
- Effectively implement right_ranks as the ceiling function (np.ceil has the bad feature that it returns float arrays)
- Revise left_ranks too to better deal with situations in which entries of left_limits are negative. As of now, we would make the corresponding left_rank equal to 0, but this means that the zero-th point is skipped, which I believe is not the ideal behaviour.
Fix incorrect definition of only_one_pt in _find_interval_limits
4467cc4 
Also, opt to subtract 1 from the definition of X_rank for code efficiency
Remove print statement
c83ee5e
This was referenced Feb 4, 2020
Closed
Closed
@wreise wreise marked this pull request as ready for review February 4, 2020 12:31
@wreise wreise changed the title [WIP] Mapper tests Mapper tests Feb 4, 2020
@wreise wreise requested a review from ulupo February 6, 2020 08:56
@wreise
Copy link
Collaborator Author

wreise commented Feb 6, 2020

The tests are failing because of check_is_fitted :

gtda/mapper/tests/test_cover.py:209: in test_cubical_fit_A_transform_consistent_with_OneD
    x_cubical = cubical.fit(filter).transform(filter)
gtda/mapper/cover.py:466: in transform
    check_is_fitted(self)

@ulupo
Copy link
Collaborator

ulupo commented Feb 6, 2020

@wreise now fixed -- but let's wait for tests to pass.

@ulupo
Copy link
Collaborator

ulupo commented Feb 6, 2020

This looks great! Aside from the comment above, the only other thing I'd ask is a one-liner description for the tests, especially those with names which may be slightly mysterious to the uninitiated to hypothesis.

ulupo
ulupo requested changes Feb 6, 2020
View reviewed changes
gtda/mapper/visualization.py Show resolved Hide resolved
gtda/mapper/tests/test_cluster.py Outdated Show resolved Hide resolved
gtda/mapper/tests/test_cluster.py Outdated Show resolved Hide resolved
gtda/mapper/tests/test_cluster.py Outdated Show resolved Hide resolved
gtda/mapper/tests/test_cluster.py Show resolved Hide resolved
gtda/mapper/tests/test_cover.py Outdated Show resolved Hide resolved
@ulupo
Copy link
Collaborator

ulupo commented Feb 6, 2020 •
edited
Loading

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wreise
Copy link
Collaborator Author

wreise commented Feb 7, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wreise
Copy link
Collaborator Author

wreise commented Feb 7, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

ulupo
ulupo requested changes Feb 7, 2020
View reviewed changes
gtda/mapper/visualization.py Outdated Show resolved Hide resolved
ulupo
ulupo requested changes Feb 7, 2020
View reviewed changes
gtda/mapper/tests/test_cover.py Outdated Show resolved Hide resolved
@ulupo ulupo changed the title Mapper tests Create more Mapper tests, and fix (#227) Feb 7, 2020
@ulupo ulupo merged commit 6fa1a58 into giotto-ai:master Feb 7, 2020
@wreise wreise deleted the mapper_tests branch April 18, 2020 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulupo ulupo ulupo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wreise @ulupo