persim icon indicating copy to clipboard operation
persim copied to clipboard

Fix pl plotting

Open catanzaromj opened this issue 3 years ago • 4 comments

This PR fixes numerous bugs with plotting persistence landscapes. In particular

  • user specified labels for coordinate axes now work,
  • doc strings are cleaned up and now on publicly accessible methods,
  • added a depth_range parameter so a subset of landscape functions can be plotted,
  • padding previously incorrectly changed landscape function values,
  • updated 3d axes call to remove deprecation error.

A max_depth attribute has also been added to PersLandscapeApprox and the notebooks were updated.

Please pass along any comments you have. Thanks.

catanzaromj avatar Jan 12 '22 14:01 catanzaromj

Codecov Report

Attention: 46 lines in your changes are missing coverage. Please review.

Comparison is base (e1fb188) 75.84% compared to head (7da61f4) 74.52%.

Files Patch % Lines
persim/landscapes/visuals.py 0.00% 46 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
- Coverage   75.84%   74.52%   -1.32%     
==========================================
  Files          20       20              
  Lines        1453     1480      +27     
  Branches      315      327      +12     
==========================================
+ Hits         1102     1103       +1     
- Misses        293      319      +26     
  Partials       58       58              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 16 '22 20:01 codecov[bot]

Hi @catanzaromj, these changes look great. Once the tests are passing we can merge. Though it looks like the test failure might be unrelated, you should be able to fix it using numpy.testing.assert_almost_equal.

Thank you!

sauln avatar Jan 30 '22 21:01 sauln

@sauln No problem. Let me know if there's anything else. Thanks!

catanzaromj avatar Feb 02 '22 20:02 catanzaromj

@sauln Is there anything else needed?

catanzaromj avatar Feb 28 '22 19:02 catanzaromj

The test coverage dropping will be covered in a future PR.

catanzaromj avatar Feb 11 '24 21:02 catanzaromj