ENH: Enable short_caption in to_latex #35668

ivanovmg · 2020-08-11T11:51:27Z

closes ENH: short caption for LaTeX tables #36267
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Enable short_caption for DataFrame.to_latex by expanding the meaning of kwarg caption.
Optionally caption can be a Tuple[str, str] = full_caption, short_caption.

The final caption macro would look like this:

\caption[short_caption]{full_caption}

Method _compose_caption_and_label_macro unifies creation of caption and label macros for both tabular and longtable envs.

Kwarg short_caption allows one to add short caption to LaTeX \caption macro. The final caption macro would look like this: ``` \caption[short_caption]{caption} ```

simonjayhawkins

Thanks @ivanovmg for the PR. needs a release note (enhancements in 1.2)

simonjayhawkins · 2020-08-11T14:02:14Z

pandas/core/generic.py

+        short_caption : str, optional
+            The LaTeX short caption.
+            Full caption output would look like this:
+            ``\caption[short_caption]{caption}``.


needs a versionadd tag

Should I put this?

.. versionadded:: 1.2.0

I was not sure about the version.

simonjayhawkins · 2020-08-11T14:04:28Z

pandas/core/generic.py

@@ -2966,7 +2967,10 @@ def to_latex(
            The LaTeX caption to be placed inside ``\caption{}`` in the output.

            .. versionadded:: 1.0.0
-
+        short_caption : str, optional


rather than another keyword parameter, maybe caption could be changed to accept Optional[Union[str, Tuple[str,str]]] to allow the short caption to be passed.

I can do you this if you think this is the best way.
My concern is that in this case caption kwarg will be too long (another concern is that further unpacking into caption and short_caption from either string or tuple will look imperfect, but it is manageable).

@simonjayhawkins, can you please confirm that this is the option to proceed with?

i've added the api design and needs discussion labels. we should get some opinions from others before proceeding. Changing the api after the event is not so easy. cc @WillAyd @jreback @toobaz

I wouldn't bother about the caption kwarg being too long, while I do find that the additional argument might be slightly easier to document. But all in all I have a slight preference for avoiding the new argument (and the new error message) and resorting to the tuple version.
I would actually phrase the docs as something like "a tuple (short_caption, full_caption), which will result in \caption[short_caption]{caption}; if a single string is passed, no short caption will be set".

Should I wait for the decision from the remaining stakeholders?

I don't have a strong preference. Looks like everyone so far has preferred the tuple version though so I'd say go ahead with that

There is some ambiguity as to whether "xy" is a single caption or two if you unpack. Maybe not a big deal but worth calling out

I made the changes required, including testing corner cases like "xy" caption.

There was a discussion after pull request in regards to the new kwarg ``short_caption``. It was decided not to introduce the new kwarg into method ``pd.DataFrame.to_latex()``, but rather optionally unpack caption into a tuple (caption, short_caption). So, if caption = str, then short_caption is None and caption macros will look like this: ``` \caption{caption} ``` If caption = (long_caption, short_caption), then caption macros will look like this: ``` \caption[short_caption]{caption} ```

ivanovmg · 2020-08-18T15:49:54Z

For some reason CI / Checks (pull_request) failed with the typing validation.

Run source activate pandas-dev
mypy --version
mypy 0.730
Performing static analysis using mypy
pandas/core/generic.py:3032: error: Argument 1 to "DataFrameFormatter" has incompatible type "NDFrame"; expected "DataFrame"  [arg-type]
Found 1 error in 1 file (checked 1030 source files)
Performing static analysis using mypy DONE
##[error]Process completed with exit code 1.

But I never touched DataFrameFormatter myself. Can you help?

WillAyd · 2020-08-19T13:16:04Z

pandas/core/generic.py

@@ -2887,7 +2887,8 @@ def to_latex(
        multicolumn=None,
        multicolumn_format=None,
        multirow=None,
-        caption=None,
+        caption: Optional[Union[str, Tuple[str, str]]] = None,
+        short_caption=None,


This can be removed from the signature right?

Did that, but the build failed due to exceeding time of 1 hour.

Failure of TestRegistration.test_pandas_plots_register, which does not seem to be related to the commits.

ivanovmg · 2020-08-26T06:50:19Z

Please put this PR on hold before the PR related to full refactoring of latex.py is merged #35872 (unless abandoned).
It will be easy to implement the changes of the present PR if #35872 is merged first.

jreback · 2020-09-08T00:28:00Z

pandas/core/generic.py

@@ -3123,6 +3124,18 @@ def to_latex(
        if multirow is None:
            multirow = config.get_option("display.latex.multirow")

+        if caption:


instead of doing this here, can you do it in the formatter itself (in the constructor is ok)

jreback · 2020-09-08T00:28:46Z

pandas/core/generic.py

@@ -3074,11 +3074,12 @@ def to_latex(
            centered labels (instead of top-aligned) across the contained
            rows, separating groups via clines. The default will be read
            from the pandas config module.
-        caption : str, optional
-            The LaTeX caption to be placed inside ``\caption{}`` in the output.
+        caption : str, tuple, optional


str or tuple, optional (change the signature as well)
Optional[Union[str, Tuple[str, str]]]

Done. I updated typing on caption in DataFrameFormatter.to_latex() and LatexFormatter.

jreback · 2020-09-08T00:28:54Z

pandas/io/formats/format.py

@@ -931,6 +931,7 @@ def to_latex(
        multicolumn_format: Optional[str] = None,
        multirow: bool = False,
        caption: Optional[str] = None,
+        short_caption: Optional[str] = None,


WillAyd · 2020-09-08T16:45:07Z

pandas/io/formats/latex.py

+                    self._caption, self.short_caption = caption
+                except ValueError as err:
+                    msg = "caption must be either str or tuple of two strings"
+                    raise ValueError(msg) from err


Can you add a test that hits this?

It is there, if you mean raising ValueError.

String 561.

# test that wrong number of params is raised with pytest.raises(ValueError): df.to_latex(caption=(the_caption, the_short_caption, "extra_string"))

I updated the test to ensure that the error message matches expectations.

ivanovmg · 2020-09-08T16:56:50Z

I guess, I need to add release notes into Enhancements section, as @simonjayhawkins highlighted. Would it be v1.2.0?

WillAyd · 2020-09-08T17:00:15Z

Correct - 1.2 whatsnew

ivanovmg · 2020-09-14T21:20:37Z

@jreback, @WillAyd, @simonjayhawkins, @toobaz, ping. Is it good to go?

WillAyd · 2020-09-21T21:44:00Z

pandas/tests/io/formats/test_to_latex.py

+"""
+        assert result_cl == expected_cl
+
+        # test when the caption, the short_caption and the label are provided


Same comment here - modular tests are much easier to debug in case of future issues

I absolutely agree. I would prefer this #36528 to be merged first, and then rebase on top of that with the separate tests.

Split tests.

WillAyd · 2020-09-21T21:44:19Z

pandas/tests/io/formats/test_to_latex.py

+"""
+        assert result_cl == expected_cl
+
+        # test when the short_caption is provided alongside caption


Can you split each comment here into a separate test?

WillAyd · 2020-09-21T21:46:09Z

pandas/io/formats/latex.py

        self.multicolumn = multicolumn
        self.multicolumn_format = multicolumn_format
        self.multirow = multirow
-        self.caption = caption
+        self.caption = caption  # type: ignore[assignment]


I am a little confused by this though - are you sure this required? str should be fine on the right hand side of an assignment based off of the comment.

jreback · 2020-09-21T21:55:52Z

pandas/io/formats/latex.py

@@ -41,6 +41,12 @@ def __init__(
        self.multirow = multirow
        self.clinebuf: List[List[int]] = []
        self.strcols = self._get_strcols()
+
+        # Here is a reason for ignoring typing.


what is the output of _get_strcols()?

_get_strcols() returns List[List[str]], where the inner list contains the elements of the column.

jreback · 2020-09-21T21:57:35Z

pandas/io/formats/latex.py

@@ -657,7 +706,29 @@ def _select_builder(self) -> Type[TableBuilderAbstract]:
        return TabularBuilder

    @property
-    def column_format(self) -> str:
+    def caption(self) -> str:


is there a reason we use setters / getters rather than just typing in teh class? seems much simpler.

Seems that there is no particular reason as the class is supposed to be initialized only once. If this is so, then I will update this into a function, which is run at init.

Test for longtable with shorcaption was changed to reflect the recent changes on master branch. The changes in longtable environment were introduced recently, see GH pandas-dev#34360.

WillAyd · 2020-09-25T15:15:14Z

pandas/io/formats/latex.py

+            \caption{caption_string}.
+        """
+        if self.caption:
+            return f"\\caption{self._short_caption_macro}{{{self.caption}}}"


Suggested change

return f"\\caption{self._short_caption_macro}{{{self.caption}}}"

return f"\\caption{self._short_caption or ''}{{{self.caption}}}"

This can be simplified to get rid of the property

short_caption_macro is either an empty string or [short caption text] (in square brackets).
Putting this logic into f-string seems to be rather complicated.
IMHO having a separate property (_short_caption_macro) with clear intent is more readable.

@WillAyd, does my answer make sense?
@jreback, @simonjayhawkins, @toobaz, any plans on merging this?
Or does it require additional work?

@ivanovmg am looking now, we have 200 open PRs; these take time

the way you have it is ok; the f-string actually is pretty complicated to grok on the substitutions. if you can make it simpler would take (e.g. use concatenation maybe)

I made string joining (and substitution in place). And removed property _short_caption_macro as @WillAyd suggested. Would you consider it more readable?

WillAyd · 2020-09-25T15:15:26Z

pandas/io/formats/latex.py

+            return f"\\caption{self._short_caption_macro}{{{self.caption}}}"
+        return ""
+
+    @property


See above - I think can remove this altogether

jreback

small comments, ping on green

jreback · 2020-10-07T03:17:58Z

pandas/io/formats/latex.py

+                    msg = "caption must be either a string or a tuple of two strings"
+                    raise ValueError(msg) from err
+        else:
+            long_caption = ""


can you make this a free function

I moved this function to module level.

jreback · 2020-10-07T03:19:02Z

pandas/io/formats/latex.py

+            \caption{caption_string}.
+        """
+        if self.caption:
+            return f"\\caption{self._short_caption_macro}{{{self.caption}}}"


the way you have it is ok; the f-string actually is pretty complicated to grok on the substitutions. if you can make it simpler would take (e.g. use concatenation maybe)

ivanovmg · 2020-10-07T11:07:53Z

@jreback all checks are good except Windows py38_np18, which failed to create anaconda environment.

pandas/io/formats/latex.py

ivanovmg · 2020-10-17T15:20:17Z

All checks are good except Windows:

##[error]We stopped hearing from agent Azure Pipelines 8.

jreback

lgtm ping on green.

jreback · 2020-10-17T16:22:40Z

doc/source/whatsnew/v1.2.0.rst

+   print(table)
+
+Usage of keyword ``caption`` is extended.
+Besides taking a single string as an argument,


I think we could add a link to to_latest here: https://pandas.pydata.org/docs/user_guide/io.html (followon as prob need a short section as well).

jreback · 2020-10-17T16:23:37Z

thanks @ivanovmg

ivanovmg added 2 commits August 11, 2020 18:29

Extract helper method for caption & label macro

13d07ae

Method _compose_caption_and_label_macro unifies creation of caption and label macros for both tabular and longtable envs.

Enable short_caption for df.to_latex

8912c81

Kwarg short_caption allows one to add short caption to LaTeX \caption macro. The final caption macro would look like this: ``` \caption[short_caption]{caption} ```

ivanovmg changed the title ~~Feature/latex shortcaption~~ Enable short_caption in to_latex Aug 11, 2020

ivanovmg added 3 commits August 11, 2020 19:19

Replace unwanted pytest.warns with tm.assert...

3f71b46

Fix missing f-string placeholder

22d2ca4

Apply black

01152c1

simonjayhawkins reviewed Aug 11, 2020

View reviewed changes

simonjayhawkins added Enhancement IO LaTeX to_latex API Design Needs Discussion Requires discussion from core team before further action labels Aug 11, 2020

ivanovmg added 4 commits August 18, 2020 21:37

Merge branch 'master' into feature/latex-shortcaption

4725d73

Add edge cases for caption testing

a7b64c0

Pass through black

a2216a1

WillAyd requested changes Aug 19, 2020

View reviewed changes

Remove typing and short_caption from to_latex

6c93de4

ivanovmg requested a review from WillAyd August 26, 2020 06:47

ivanovmg added 2 commits September 8, 2020 03:11

Merge branch 'master' into feature/latex-shortcaption

45de6eb

DOC: add parameters to LatexFormatter docstring

e70aafa

jreback requested changes Sep 8, 2020

View reviewed changes

ivanovmg added 2 commits September 8, 2020 12:03

TYP: remove type ignore for column_format

a0e3f53

REF: move short caption parsing to LatexFormatter

6725ca8

ivanovmg requested a review from jreback September 8, 2020 06:41

WillAyd reviewed Sep 8, 2020

View reviewed changes

ivanovmg requested a review from WillAyd September 8, 2020 16:51

WillAyd requested changes Sep 21, 2020

View reviewed changes

jreback requested changes Sep 21, 2020

View reviewed changes

ivanovmg added 5 commits September 22, 2020 15:30

REF: replace caption setter with initialize method

ae1babe

Merge branch 'master' into feature/latex-shortcaption

ddec116

TST: align longtable test with the recent changes

15227b3

Test for longtable with shorcaption was changed to reflect the recent changes on master branch. The changes in longtable environment were introduced recently, see GH pandas-dev#34360.

Merge branch 'master' into feature/latex-shortcaption

336bfb5

TST: add for list [full_caption, short_caption]

b30d2d7

ivanovmg requested review from jreback and WillAyd September 23, 2020 10:57

WillAyd requested changes Sep 25, 2020

View reviewed changes

ivanovmg added 2 commits September 25, 2020 23:04

Merge branch 'master' into feature/latex-shortcaption

f1781f6

Merge branch 'master' into feature/latex-shortcaption

f4b18ff

jreback requested changes Oct 7, 2020

View reviewed changes

ivanovmg added 4 commits October 7, 2020 15:17

Merge branch 'master' into feature/latex-shortcaption

a18c85d

REF: use string concat for caption macro

e957c37

REF: move method to module level

09d9c85

REF: drop property _short_caption_macro

559ca2a

ivanovmg requested a review from jreback October 7, 2020 11:08

jreback requested changes Oct 10, 2020

View reviewed changes

pandas/io/formats/latex.py Show resolved Hide resolved

ivanovmg requested a review from jreback October 12, 2020 19:36

Merge branch 'master' into feature/latex-shortcaption

0fd73e8

jreback approved these changes Oct 17, 2020

View reviewed changes

jreback merged commit f1b2bb1 into pandas-dev:master Oct 17, 2020

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Oct 26, 2020

ENH: Enable short_caption in to_latex (pandas-dev#35668)

98ec8eb

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

ENH: Enable short_caption in to_latex (pandas-dev#35668)

356262d

ivanovmg deleted the feature/latex-shortcaption branch November 6, 2020 15:34

	return f"\\caption{self._short_caption_macro}{{{self.caption}}}"
	return f"\\caption{self._short_caption or ''}{{{self.caption}}}"

ENH: Enable short_caption in to_latex #35668

ENH: Enable short_caption in to_latex #35668

Conversation

ivanovmg commented Aug 11, 2020 • edited Loading

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg commented Aug 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg commented Aug 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg commented Sep 8, 2020

WillAyd commented Sep 8, 2020

ivanovmg commented Sep 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg Sep 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg commented Oct 7, 2020

ivanovmg commented Oct 17, 2020

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 17, 2020

ivanovmg commented Aug 11, 2020 •

edited

Loading

ivanovmg Sep 23, 2020 •

edited

Loading

ivanovmg Oct 7, 2020 •

edited

Loading