Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two proposals for estimation_table #373

Closed
ChristianZimpelmann opened this issue Aug 1, 2022 · 3 comments · Fixed by #379
Closed

Two proposals for estimation_table #373

ChristianZimpelmann opened this issue Aug 1, 2022 · 3 comments · Fixed by #379
Labels
enhancement New feature or request

Comments

@ChristianZimpelmann
Copy link
Member

  • estimagic version used, if any: main branch
  • Python version, if any: 3.9.10
  • Operating System: Windows

What would you like to enhance and why? Is it related to an issue/problem?

Transitioning to the new estimagic tables worked out very well, thanks! But two issues appeared:

  • I didn't find an easy way to add a row of strings (e.g. "Controls": ["Yes", "No"]) to the footer before calling render_latex. I think it would be a good feature if the function would allow for these simple strings and wrap them as f"\\multicolumn{{1}}{{c}}{{{sr[i]}}}" automatically. Alternatively, I could add the multicolumn environments as the user myself. But this didn't work because an additional bracket was added by the function. I started to implement the former solution on the branch improve_estimation_table and can make it nicer in case you aggree with the general idea.
  • The row showing observations is now also rounded (in my case to two significant digits), but I would like to not have it rounded -- preferably not only if it is called "observations" (maybe even for all integer rows). This feature was implemented in a previous version of estimagic, but seems to be missing now. What are your thoughts about it?
@ChristianZimpelmann ChristianZimpelmann added the enhancement New feature or request label Aug 1, 2022
@janosg
Copy link
Member

janosg commented Aug 4, 2022

Hi Christian,

Thanks a lot!

  1. Yes, adding string rows should me made easy for users. If you have a good implementation we would be happy to receive a PR.
  2. I thought it was implemented that ints are displayed correctly. I don't think we should round them at all by default. People will get confused if our number of observations is not exactly the dataset size. @mpetrosian can you check? @ChristianZimpelmann do you have a minimal example where it goes wrong?

@hmgaudecker
Copy link
Member

I thought it was implemented that ints are displayed correctly. I don't think we should round them at all by default. People will get confused if our number of observations is not exactly the dataset size.

I might be completely off here, but I the impression that I got was that the logic is very much column-based (too much IMHO for estimation tables which tend to be a weird collection of all kinds of numbers having little to do with each other apart from being based on the same regression or whatever). That is, if I have three rows, the first two are regression coefficients of very different magnitudes and the last is an integer with the number of observations, this does not quite work.

@mpetrosian
Copy link
Member

Hi.
so, we implement unfomratting of integers which removes the trailing zero. This means that the number can get affected by rounding. in principle, this checking for integer numbers can be done at an earlier step such that the numbers don't get formatted.

re the first point: You can start a PR and i can pick up on it later.

In the next couple of weeks, I won't be able to work on estimation_table, thought. Hope this is fine with everyone!.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants