Becksteinlab / kda

Python package used for the analysis of biochemical kinetic diagrams.
GNU General Public License v3.0
3 stars 1 forks source link

MAINT: Simplify plotting module #45

Closed nawtrey closed 2 years ago

nawtrey commented 2 years ago

Description

Refactor of the primary functions in plotting.py.

Details

codecov[bot] commented 2 years ago

Codecov Report

Merging #45 (7425fc6) into master (3511db2) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   99.86%   99.86%   -0.01%     
==========================================
  Files           9        9              
  Lines         747      724      -23     
==========================================
- Hits          746      723      -23     
  Misses          1        1              
Impacted Files Coverage Δ
kda/plotting.py 100.00% <100.00%> (ø)
nawtrey commented 2 years ago

The code coverage has dropped as a result of the code simplification, which is expected (and acceptable). There is still only 1 uncovered line across the code base, which is good.

nawtrey commented 2 years ago

Upon merging a few issues will have to be opened to address some remaining changes:

  1. the documentation for the new and refactored functions needs to be updated
  2. draw_cycles(panel=True) can likely be further simplified via _plot_panel()
  3. A single function can probably be written to handle both cycles and diagrams. Since their structures are so similar, it shouldn't be too difficult to create a single function to handle either of those cases.
  4. Add unit tests for each of the plotting helper functions
  5. Change "label" parameter to "filename", and don't include the file extensions
  6. Remove "single diagram/cycle" cases and change to enforcing an array or list as input since that code seems to be very redundant and largely unnecessary. It's nice to be able to input a single diagram, but the names draw_diagrams and draw_cycles are probably a bit misleading in the first place. Might be more appropriate to make a public function for plotting individual diagrams/cycles.
nawtrey commented 2 years ago

Overall these changes substantially improve the readability of the code, reduce many instances of repeated code, and still maintain the functionality of the code. The test suite also gets a makeover that allows much better error detection.

I've given the code a good look again, and tried all of the various plotting combinations out. Everything looks good to me, and CI is green, so merging.