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

Improve table printing #4

Open
DominiqueMakowski opened this issue Feb 17, 2019 · 15 comments
Open

Improve table printing #4

DominiqueMakowski opened this issue Feb 17, 2019 · 15 comments
Labels
feature idea 🔥 New feature or request help us 👀 Help is needed to implement something

Comments

@DominiqueMakowski
Copy link
Member

Table printing is a bit jiggly, i.e., the columns are not perfectly aligned (due to coloured columns). This must be improved, but I am not sure how...

The answer is probably within the .display.data.frame function in display.R, where the alignment is done.

@strengejacke
Copy link
Member

I digged a bit into the code, and this one is tricky... we need to use format() on the complete column, e.g. in .colour_column_if(). But once it's a string, we can't check for the negative / positive sign... ?!?

@DominiqueMakowski
Copy link
Member Author

Yes, I tried with some regex detection (if the stripped characters began with the "-" string), without success...

We might need to imagine another way of integrating the colouring, the formatting and the to-string conversion than my clunky proposal

@DominiqueMakowski
Copy link
Member Author

Moreover, the format has to be used on the striped columns (without the colour codes), otherwise, the columns are too large (as the colour codes disappear)... So yea very tricky, puzzling and challenging haha... and worth trouble (even now with the uneven columns, I find the colouring of the coefs and such super cool 😄)

strengejacke added a commit that referenced this issue Feb 17, 2019
@strengejacke
Copy link
Member

Ok, I think we can reduce code complexity and should put the formatting in .colour_column_if(). Now we have to get rid of NA where it makes no sense.

@strengejacke
Copy link
Member

unbenannt

@DominiqueMakowski
Copy link
Member Author

This can be relevant for future vignette / readme improvements: https://twitter.com/BrodieGaslam/status/1029059002225721344

@strengejacke
Copy link
Member

I would say that column with numers should be right-aligned, while "string" columns (with values that are non-numeric from the user's perspective) should be left-aligned... Isn't this a common table convention?

@strengejacke
Copy link
Member

Table printing no longer works for model-objects when digits are specified. Thus, we need some extra handling here. I tried to solve this using an additional class attribute. We should think about how to solve this issue, respectively if my suggestion is ok and not too sloppy.

@DominiqueMakowski
Copy link
Member Author

image

It looks great 🎊 🎉 ! Brilliant implementation, thanks :)

@DominiqueMakowski
Copy link
Member Author

It seems that bold slightly enlarges the column, resulting in a minor misalignment toward the end... but that's details ^^

@strengejacke
Copy link
Member

Damn... ;-)
I have a fix in mind, will try this evening. We could, for the p-value column, replace all empty values with as many whitespaces as "column-width" (nchar()) and sourround the whitespaces with bold as well... Or we switch to a colour. But misalignment doesn't look that nice. :-)

strengejacke added a commit that referenced this issue Feb 18, 2019
@strengejacke
Copy link
Member

Can you check if my workaround works for you? I don't have this alignment issue.

@DominiqueMakowski
Copy link
Member Author

image

Seems like it's still there... are you using R studio also?

@strengejacke
Copy link
Member

Yes, I'm using RStudio (both latest stable and current preview) on Windows 10 (light theme). Strange, but then let's format p-values in colour, not bold.

@DominiqueMakowski
Copy link
Member Author

Strange indeed... maybe has something to do with the font used?
Will change back to yellow then (or maybe orange if yellow is too hard to read on a light theme?)

@DominiqueMakowski DominiqueMakowski added feature idea 🔥 New feature or request help us 👀 Help is needed to implement something labels Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature idea 🔥 New feature or request help us 👀 Help is needed to implement something
Projects
None yet
Development

No branches or pull requests

2 participants