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

Restrict output height of PCA model representation #195

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wildart
Copy link
Collaborator

@wildart wildart commented May 28, 2022

Related to #186.

@nalimilan
Copy link
Member

How about mentioning that dimensions are not printed as they don't fit on screen? It could make sense to print the command that allows printing all results, as it's not easy to find.

@wildart
Copy link
Collaborator Author

wildart commented Jun 10, 2022

It could make sense to print the command that allows printing all results, as it's not easy to find.

Good idea, will do.

@FedeClaudi
Copy link

Was adding a comment on the original issue (#186) but will add my 2 cents here instead.

First, thanks for the package, awesome work.

For the layout problem raised in the issue, I would suggest Term's Tables for styled terminal output. Disclaimer: I'm Term's developer so I'm a bit biased. I'm happy to put together a PR showing how that would look.


I also have an additional gripe with the current output. Currently, it prints out a whole lot of information (not sure if it's the data it was fitted on, or the PC loadings). That's fine for small examples, but I forgot to add ; while fitting a HUGE table and it has been printing values to the console for like 5 minutes now, still going. Realistically, one would not visually inspect large tables of numbers like that, I really don't see the point of printing it out. Also, there's methods to easily display that information if one really wants to have a look, but I don't think it should be the default.

Looking forwards to seeing what you think!

@wildart
Copy link
Collaborator Author

wildart commented Sep 4, 2022

For the layout problem raised in the issue, I would suggest Term's Tables for styled terminal output.

The table layout and handling comes from StatsBase.CoefTable, so the work needs to be done there.

there's methods to easily display that information if one really wants to have a look, but I don't think it should be the default.

In #195, I put several limits on default output, but main work needs to be done in CoefTable - make it adjustable (see JuliaStats/StatsBase.jl#794).

Thanks for comments.

@FedeClaudi
Copy link

Got it, thanks for the reply and the excellent work!

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

Successfully merging this pull request may close these issues.

None yet

3 participants