CLN: io/formats/html.py: refactor
State: closed

- [N/A ] follow on from #20341
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [N/A ] whatsnew entry




Labels:
name: Refactor
description: Internal refactoring of code
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
Hello @simonjayhawkins! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`pandas/io/formats/html.py`](https://github.com/simonjayhawkins/pandas/blob/63605b295b0fa5aeb4485f05d00240a2801372b6/pandas/io/formats/html.py) !


@simonjayhawkins : Not sure where this was discussed before, but could you explain in your PR description why making `indent` a stateful entity makes sense than leaving it as a parameter to individual methods?
@gfyoung : good question. why change?

at the moment this was just an exercise to see if the current behavior could be implemented in a simpler way. why?  

was thinking what would the code look like if the common behavior of to_string()/DataFrameFormatter and to_html()/HTMLFormatter were inherited from TableFormatter.

was also thinking how would the code look if to_html(notebook=True) were a subclass of HTMLFormatter, say NotebookFormatter and to_html(notebook=False) were also a subclass, say ToHTMLFormatter.

i think this would allow easier implementation of some of the enhancements commonly requested for the to_html() method without making potentially breaking changes to the notebook display code path.
for example, #22692 just in the last few days, 

with regard to fine grain control of the indentation on a method level, another subclass of HTMLFormatter maybe. but that is not needed at the moment as the current indentation behavior can be replicated without it.

i'll report back when i have explored this avenue further. 




> i'll report back when i have explored this avenue further.

Sounds good.  Might have preferred that an issue be filed first, but admittedly, the discussion would have been hard to carry without something concrete to analyse.
quote from #20341

> ... PR splits the various formatters into different files so that it's easier to refactor a specific formatter.


@gfyoung : i've reverted the changes to indentation for the moment and added some simpler refactorings to start.
does this actually change any output?
@jreback : Are you referring to the comment regarding `indent_delta`, or the removal of the unreachable code and subsequent refactoring? The answer is no and no.

I've added some additional tests, but not all paths can be tested until the fixes for #22655 have been implemented.

All the tests in `pandas\tests\io\formats\test_to_html.py` currently test the full `df.to_html()` path. Adding additional tests is quite time consuming. I still need to add some more tests to #22655 before I'm happy that the permutations of input parameters with DataFrame truncation has been fully tested.

maybe the way forward is to add unit tests for the `HTMLFormatter` methods?  
lgtm. @WillAyd if you'd have a quick look.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=h1) Report
> Merging [#22726](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/1250500cfe18f60cfb8a4867c82d14467bbee7ad?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/22726/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #22726      +/-   ##
==========================================
+ Coverage   92.23%   92.26%   +0.02%     
==========================================
  Files         161      161              
  Lines       51408    51387      -21     
==========================================
- Hits        47416    47410       -6     
+ Misses       3992     3977      -15
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.65% <100%> (+0.02%)` | :arrow_up: |
| #single | `42.32% <0%> (+0.01%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/formats/html.py](https://codecov.io/gh/pandas-dev/pandas/pull/22726/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvaHRtbC5weQ==) | `97.68% <100%> (+4.49%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=footer). Last update [1250500...0905033](https://codecov.io/gh/pandas-dev/pandas/pull/22726?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

can you merge master on this.
@WillAyd comments?
Been a little tight for time recently but should be able to review again before the end of the week. @simonjayhawkins if you could merge master in the interim would be helpful
thanks @simonjayhawkins