Custom lines at various locations #14
Conversation
csemken
force-pushed
the
custom_text_generic
branch
from
08f4530
to
df54835
Compare
csemken
force-pushed
the
custom_text_generic
branch
from
df54835
to
9d1141f
Compare
|
I'd love to use this functionality! Could a maintainer please weigh in on which solution between this and #13 (or something else) is preferred, so conflicts can be resolved and one of them can be merged? |
|
+1. Let me know whether you prefer this or #13 and I can update the PR. |
|
@csemken @MaxGhenis I think this more flexible approach is nice to have. Few comments:
|
stargazer/stargazer.py
Outdated
| @@ -176,6 +178,14 @@ def reset_covariate_order(self): | |||
| if self.original_cov_names is not None: | |||
| self.cov_names = self.original_cov_names | |||
|
|
|||
| def add_custom_lines(self, lines, location='footer top'): | |||
There was a problem hiding this comment.
+1 to @toobaz's comment, I think add_line would be more natural.
Also lines seems like an odd term, isn't this just one line that's added at a time? Below when you loop through the elements, those are individual values right?
stargazer/stargazer.py
Outdated
| def add_custom_lines(self, lines, location='footer top'): | ||
| for line in lines: | ||
| assert len(line) == self.num_models + 1, \ | ||
| 'Please input iterables of length "number of models + 1"' |
There was a problem hiding this comment.
A separate label arg may be cleaner than requiring it to be self.num_models + 1. This would also be more future-proof, if stargazer eventually supports multiple left-most (non-model) columns.
There was a problem hiding this comment.
Maybe we can allow both approaches: check that line has self.num_models + 1 (where 1 would be possibly replaced in the future by the number of left headers) items, and only if it has self.num_models instead, use the (optional) label argument (which would default to ''). (Raise if it has neither number of items)
There was a problem hiding this comment.
Great idea. I think only supporting line of length self.num_models with a label would be the cleanest option. That way we offer one clear interface. Offering both interfaces might be confusing, increases code complexity and still creates the multiple left-most columns problem that @MaxGhenis mentioned.
There was a problem hiding this comment.
Great idea. I think only supporting
lineof lengthself.num_modelswith alabelwould be the cleanest option.
OK, no objection: then I would have label as first argument and line as second.
stargazer/stargazer.py
Outdated
| 'Please input iterables of length "number of models + 1"' | ||
| assert location in self.CUSTOM_LINES_LOCATIONS, \ | ||
| 'location needs to be one of {}'.format(str(self.CUSTOM_LINES_LOCATIONS)) | ||
| self.custom_lines[location] = lines |
There was a problem hiding this comment.
This overwrites the current value, so only one line can be added per location, right?
It'd be nice to be able to add multiple lines per location, such that this would be some sort of append statement.
There was a problem hiding this comment.
It'd be nice to be able to add multiple lines per location, such that this would be some sort of append statement.
Good point: it would be great if each self.custom_lines[location] was a list, and add_line appended lines to it. Then (at least for now) we could just mention in the docstrings that one can remove all added lines with table.custom_lines[location] = [].
| for custom_row in self.custom_lines[location]: | ||
| custom_text += '<tr><td style="text-align: left">' + str(custom_row[0]) + '</td>' | ||
| for custom_column in custom_row[1:]: | ||
| custom_text += '<td>' + str(custom_column) + '</td>' |
There was a problem hiding this comment.
This str should in principle be replaced by the appropriate formatters for float (_float_format) and some day, int (see #53); ideally, add_line should even have an optional formatter argument. But all of this can be done later in a separate PR to now slow down this one.
|
Excellent suggestions @toobaz and @MaxGhenis.
Would you prefer an enum?
Do you have any docstring format preferences? Maybe Google? |
Yes, good idea. So users can cleanly import them, and they can always use the numbers if they prefer.
I guess we could go with Numpydoc, as it is used by pandas, statsmodels... most of the stack stargazer users are probably used to. |
https://pandas.pydata.org/pandas-docs/stable/development/contributing_docstring.html |
csemken
force-pushed
the
custom_text_generic
branch
from
27338be
to
6a59cf0
Compare
|
All done. |
|
Could you add to |
|
Great @MaxGhenis! I am glad it is useful.
Absolutely. I updated both @toobaz Ready to merge? |
This request is based on and implements the suggestions I made in #13 (only one should be accepted).
It allows users to add custom lines to various parts of the table, which are defined in a constant:
CUSTOM_LINES_LOCATIONS = ['body bottom', 'footer top', 'footer bottom']Thus, users can, for example, create the following common regression table:
where Controls is added with location='body bottom', Sample is added with location='footer top' (the default location) and Mean Dep Var is added with location='footer bottom'.
I renamed the custom_footer_text variable to custom_lines and the setter method add_custom_lines(), where 'lines' is the word used in the Stargazer R package. Since this functionality was not previously implement, the code remains backwards compatible.