search

scikit-tda / kepler-mapper

Kepler Mapper: A flexible Python implementation of the Mapper algorithm.
https://kepler-mapper.scikit-tda.org
MIT License
623 stars 180 forks source link

`test_cubes_overlap` may be faulty #233

Closed erooke closed 2 years ago

erooke commented 2 years ago

I'm not 100% sure what this test is supposed to be testing: https://github.com/scikit-tda/kepler-mapper/blob/7dc4d0ad3a9020fe51ec7a52995b9593b9090bab/test/test_coverer.py#L45-L58 Its name suggests it is making sure the cubes overlap. If that's the case then I suspect this line should say intersection instead of union https://github.com/scikit-tda/kepler-mapper/blob/7dc4d0ad3a9020fe51ec7a52995b9593b9090bab/test/test_coverer.py#L58 The test still passes with this changed, it may still be worth changing I don't know. If my understanding is correct #232 fixes this issue.

deargle commented 2 years ago

I think you're right -- it's supposed to fail if two sets don't have an intersection. Therefore, you're right, it's supposed to be intersection.

erooke commented 2 years ago

232 was merged making this a non issue