STYLE enable pylint: method-cache-max-size-none
State: closed

Issue #48855. This PR enables pylint type "W" warning: `method-cache-max-size-none`
Based on issue #47371 PERF: Improve Styler to_excel Performance I believe the warning is a false-positive in pandas/io/formats/excel.py#L176.
I disabled this warning [on the line 176](https://github.com/pandas-dev/pandas/blob/a37b78d534cd25e21d2ab64318ceae808df46c89/pandas/io/formats/excel.py#L176)

- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).

Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
Thanks @natmokval - not sure, I think this might be a case where the check might be telling us something useful

@mroeschke do you happen to remember why `max_size` was set to `None`, rather than the default `128`? https://github.com/pandas-dev/pandas/pull/47371#discussion_r908697576
The jobs' logs aren't available anymore
> @mroeschke do you happen to remember why max_size was set to None, rather than the default 128? https://github.com/pandas-dev/pandas/pull/47371#discussion_r908697576

Not too familiar with excel styling, but probably was thinking the opposite, why should the cache size be limited?

Could the caching be refactored to an independent function like in https://pylint.pycqa.org/en/latest/user_guide/messages/warning/method-cache-max-size-none.html?
> Thanks @natmokval - not sure, I think this might be a case where the check might be telling us something useful

Thanks for checking @MarcoGorelli. In case I set `max_size` to `128`, what will be the best way to check that my changes do not lead to regression?
> Could the caching be refactored to an independent function like in https://pylint.pycqa.org/en/latest/user_guide/messages/warning/method-cache-max-size-none.html?

Thanks, @mroeschke. On the surface I don’t see how the below content can be moved out of the class:
```
properties = self.compute_css(declarations, self.inherited)
return self.build_xlstyle(properties)
```
Can you please advise me if you see a direction?


If I am understanding how this is used from my cursory scan of the code, this is used in `to_excel` to CSS style each cell of the result but will be cached relative to the number of unique `Styler.ctx` + `Styler.ctx_index` + `Styler.ctx_columns`

So to justify changing maxsize from `None` to `128` (or similar), is it reasonable to expect a user to provide 100's of CSS declarations? cc @attack68  
The number of total possible CSS properties must only be 100-200. So to use them all per cell is never done. V Rare to go more that 10.

Of course potentially lots more independent declarations if going by number of cells, but I'm assuming you meant the former.
Thanks - let's just go with the default (128) and put `@lru_cache()` then?
> Thanks - let's just go with the default (128) and put @lru_cache() then?

Sounds good
> Can you please advise me if you see a direction?

I think what needs doing, then is to apply the second diff from https://github.com/pandas-dev/pandas/pull/49751#pullrequestreview-1185683790 , but:
- no need to add the `__del__` method (that was just for debugging), nor the other print statement
- the docstring of `_call_uncached` should go to the docstring of `__call__`
- above the assignment `self._call_cached = `, there should probably be a commenting linking back to this issue, explaining why we avoid `lru_cache` on the method

`pandas/tests/io/formats/test_to_excel.py` will also need adjusting slightly, please make sure the tests pass (feel free to reach out on the Slack group if anything trips you up)
Changes from the second diff are applied. `pandas/tests/io/formats/test_to_excel.py` works with the changes. My local test run passed.
Mentioned the memory leak in v1.5.2 whatsnew with issue #48855. Maybe it would be better to skip the number or create a new issue?
> Mentioned the memory leak in v1.5.2 whatsnew with issue #48855. Maybe it would be better to skip the number or create a new issue?

The PR number is fine if there's no issue open

Cool, let's wait for tests and we can ship it
Oops, something went wrong applying the patch ... Please have a look at my logs.
Thank you @MarcoGorelli and @mroeschke for your help and support. Your explanations helped me to understand the problem.