implemented additionally functionality of formatters_col in to_latex #37552
Conversation
wgranados
changed the title
pandas/core/generic.py
Outdated
| @@ -3136,6 +3136,22 @@ def to_latex( | |||
| if multirow is None: | |||
| multirow = config.get_option("display.latex.multirow") | |||
|
|
|||
| if is_list_like(formatters) and not isinstance(formatters, dict): | |||
There was a problem hiding this comment.
can you add an Example that uses this (e.g. similar to your test is fine)
There was a problem hiding this comment.
I was having an issue providing an example with lambda functions in doc, so I included a simpler example.
|
cc @ivanovmg if you'd have a look |
There was a problem hiding this comment.
I agree with Jeff on moving the formatters logic into DataFrameFormatter.
There is a method _initialize_formatters, which can be updated.
However, it will alter formatters not only for latex output, but for others as well (to_string, to_html, to_csv).
Is that ok?
pandas/core/generic.py
Outdated
| ) | ||
| if formatter_elems_type: | ||
| formatters = [ | ||
| (lambda style: lambda x: "{0:{1}}".format(x, style))(style) |
There was a problem hiding this comment.
I would suggest not to use two folded lambda functions. This is not easily readable. Can it be replaced with an f-string?
There was a problem hiding this comment.
Initially they were simpler lambda functions to take advantage of the existing callable implementation, but there was a python memory specific error where it created the first anonymous function and replicated it multiple times.
I believe this is using fstrings? At least for the substitution. But yeah I agree it's a little hard to read.
i think this is ok but we should have some additional tests in place |
Which tests did you have in mind? I think the FormattersType change looks self contained with the DataFrameFormatter class |
| if is_list_like(formatters) and not isinstance(formatters, dict): | ||
| formatter_elems_type = all( | ||
| isinstance(elem, str) or callable(elem) for elem in formatters |
There was a problem hiding this comment.
I believe the failures in tests occur because the logic added is not properly incorporated into the remaining items in this method.
There was a problem hiding this comment.
Probably should look like this:
if formatters is None:
return {}
elif len(self.frame.columns) == len(formatters) or isinstance(formatters, dict):
return formatters
elif is_list_like(formatters) and not isinstance(formatters, dict):
... and so on
Also raising ValueError should be in agreement (probably need to update the error message on what formatter elements should look like).
There was a problem hiding this comment.
With the ordering you proposed that second if statement would always be evaluated true and it wouldn't convert the fstrings to corresponding functions.
I'm not too sure how to fix the error, been looking over it slowly past few days. I think mypy is treating the two-folded lambda function as a different type (Like List[Callable[... some params]) than the intended List[Callable]. Kinda odd that it didn't have this issue before though 🤔
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
|
closing as stale. if you want to continue, pls ping and can re-open. |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffWas having some issues rebasing on #32666 so making a new PR with same changes. Closing other PR as well.