Remove matplotlib-based periodic table plotting functions

[janosh]

• Remove ptable_matplotlib.py, _process_data.py, and _projector.py
• Update __init__.py to remove matplotlib-based periodic table imports
• Update readme to focus on plotly-based periodic table visualization
• Add ptable_matplotlib.py removal note in readme recommending use of equivalent plotly functions instead

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.17%. Comparing base (502c573) to head (040c123).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   86.62%   87.17%   +0.54%     
==========================================
  Files          47       44       -3     
  Lines        5211     4591     -620     
  Branches      983      863     -120     
==========================================
- Hits         4514     4002     -512     
+ Misses        430      370      -60     
+ Partials      267      219      -48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[janosh]

@DanielYang59 i know you put a lot of work into refactoring the matplotlib ptable_* functions last year but given we now have feature parity with the plotly variants which offer interactivity and have better test coverage than their matplotlib equivalents, i think it makes sense to remove the matplotlib versions altogether.

coverage of the ptable folder

following the Zen of Python, i think it's best if there's just 1 way to plot periodic tables

There should be one-- and preferably only one --obvious way to do it.

i find plotly easier to maintain and add features to, and it's also less confusing to users if pymatviz exposes an overall smaller set of plot functions without any redundancy

[DanielYang59]

Hi @janosh thanks for asking me!

No need to worry about the sunk cost of the mpl ptable plotters, as long as plotly alternatives could completely replace their functionality even in terms of non-interactive plots (also perhaps they could be used in the future to simplify plotly plotters as it seems to me plot ptable plotters seem to have quite some code repetition in terms of laying down the structure of ptable).

I would certainly be on deck to migrate to plotly, but I'm a bit concerned about the breakage per se (as periodic table plotters in my option is a pretty "core" functionality of pmv), though I understand you usually want changes to happen fast ;)

[janosh]

ploty ptable plotters seem to have quite some code repetition in terms of laying down the structure of ptable).

that's a fair point. certainly worth trying to DRY the code at some point though from past experience, there's a significant risk of making the different plots harder to maintain from coupling them too closely

though I understand you usually want changes to happen fast ;)

you know me by now. 😅 i was going to deprecate first but then realized i wouldn't be able to increase the target test coverage in Codecov if the module is still around. maybe there's a way to exclude module from mean coverage calculations but didn't feel like researching that or adding new tests for soon-to-be-removed functionality

[DanielYang59]

i wouldn't be able to increase the target test coverage in Codecov if the module is still around. maybe there's a way to exclude module from mean coverage calculations but didn't feel like researching that or adding new tests for soon-to-be-removed functionality

I wouldn't worry too much about the number shown by codecov maybe (especially for our plotter code base)? I just had a quick glance at the coverage and it seems most misses are from type checking imports (no idea why), or warnings for deprecated args of ptable_heatmap. So shouldn't be concerning IMO.

[janosh]

I just had a quick glance at the coverage and it seems most misses are from type checking imports (no idea why)

looks like i need to check how to configure Codecov after all to exclude those lines from coverage calculating. pymatgen does it in pyproject.toml. still, since the same settings apply to the plotly modules, relative differences in coverage are still meaningful, no? even if the actual coverage on coverable lines is slightly higher in both cases