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

[Possible breaking change] Reimagine column handling #124

Closed
Nukesor opened this issue Aug 16, 2023 · 12 comments
Closed

[Possible breaking change] Reimagine column handling #124

Nukesor opened this issue Aug 16, 2023 · 12 comments
Labels
t: discussion Things that need a bit of a discussion t: enhancement Enhancing an existing feature or code

Comments

@Nukesor
Copy link
Owner

Nukesor commented Aug 16, 2023

This is the old ticket. I closed it due to a lot of unrelated discussion to the actual ticket. See #144

The current column handling is rather convenient, as columns get populated as rows and cells are added to the table.

However, this logic is rather implicit. There's no way to remove or add columns half-way through a table creation process.

To fix this

  • column creation should be made explicit
  • or it should be possible to make column creation explicit

I'm actually in favor of the second approach, since the goal of comfy-table is to be minimalistic and convenient to use.
Explicit column handling is more of an edge-case and the 90% use case is covered by implicit column creation.

However, it could be very tricky to design the API around an optionally explicit column handling, since such APIs tend to get confusing quite quickly.

This ticket is mostly for discussion on how such an API would look like.

Relevant tickets/MRs:a
#121

@arturdaraujo
Copy link

In polars repository we have an issue with the column handling. Solving this would be really great

@Nukesor
Copy link
Owner Author

Nukesor commented Aug 26, 2023

@arturdaraujo Could you elaborate on what the issue looks like?

The more use-cases we get to know, the better we can build the API around them.

@arturdaraujo
Copy link

arturdaraujo commented Aug 26, 2023

@Nukesor We are trying to dynamically pick the number of columns based on the terminal. It seems that the issue can be solved by 2 routes for polars package:

  1. We could use os.terminal_size (this solution would be just for the python version of polars) but it appears that this package has a specific problem with pycharm IDE, so it's tricky to implement it. We would need to wait for Jetbrains to fix that.

  2. The other solution would come from comfy-table package, but I'm not sure how that would work since the implementation would be in Rust. If comfy-table finds a solution for dynamically pick the number of columns based on terminal width, we could make polars package column handling as flawless as pandas.

The discussion is also occurring here pola-rs/polars#10510

@Nukesor
Copy link
Owner Author

Nukesor commented Aug 27, 2023

Pheww, interesting...

How does pandas display this case. Do they add an [...] column to show that there're other columns that aren't displayed?

@arturdaraujo
Copy link

Yes!

@Nukesor
Copy link
Owner Author

Nukesor commented Aug 28, 2023

I think the pandas case can be handled via a new flag on the table and I would handle this a as a separate feature :)

One problem I see with this: I don't think this feature will easily work in combination with the "dynamic" content arrangement mode. I.e. that mode is already super complex due to the logic that tries to find an near optimal layout for a given table width and content. We might just don't know how wide any column will actually be, since this depends on how wide other columns are. There'll be many further edge-cases and I cannot come up with a good solution from the top of my head.

For now, I would restrict this to the "default" usecase where content is just displayed without any text formatting logic. This would mean that any output is displayed as-is, but columns that would be wider than the table width will just be hinted at via a [...] column.

@arturdaraujo
Copy link

arturdaraujo commented Aug 29, 2023

But isn't there a way to determine the terminal width in rust? You could put an option that based on the terminal width, you limit the number of columns, with the minimum of 1 column always displaying. (I don't think this should be the default, but it would be very nice to have the option to do that)

Another cool feature would be to set the maximum column width to "break" to next line. This way the user could have many columns together regardless of how much text is in.

I don't think this should be the default but certainly would be a extremely useful for many packages out there. In polars package there around 5 issues just on this topic.

If you need help designing it I'm glad to help or even get more people to help (many people would like that feature on polars).

@Nukesor
Copy link
Owner Author

Nukesor commented Aug 31, 2023

Another cool feature would be to set the maximum column width to "break" to next line. This way the user could have many columns together regardless of how much text is in.

There actually is. comfy-table has different ContentArrangement modes. Dynamic takes the current terminal width and tries to the best of its abilities to find a proper layout.

Additionally, each column can have Constraints, which allow some control on how the text will flow.

The problem with this is that this is a classic optimization problem. Comfy-table tries to find a good solution, but unless users explicitly specify some constraints, all columns are treated equally.

However, this is usually not what users want. Columns might differ in importance or content length, which should result in those columns getting more space. But this is nothing that can be inferred from the content, since comfy-table accepts completely arbitrary content. The context needs to be provided by the user.

To give a more in-depth explanation of my previous comment:

One problem I see with this: I don't think this feature will easily work in combination with the "dynamic" content arrangement mode

This is the core logic that tries to find a proper layout in "dynamic" mode.
The main purpose of the "dynamic" mode is to split/format the contents of all columns in a way, that the currently available space is used as good as possible.

But if we were to also dynamically hide columns, this logic would no longer work, since there's nothing we can optimize for. Users would have to define some kind of cut-off value. A minimum width for each column (i.e. a min constraint).
From what I understand though, this is not really a possibility for polars, since it's used to display arbitrary data, making a global default cut-off infeasible.

For now, I would restrict this to the "default" usecase where content is just displayed without any text formatting logic.

Building the feature for the "non-dynamic" case would be rather easy. We would just hide columns that don't fit into the current width.

I guess it would be doable for the "dynamic" case as well, as long as all columns have a LowerBoundary ColumnConstraint, since we could then just run the same logic as in the "non-dynamic" case.

However, I don't see a way to implement this without the explicit definition of lower boundary constraints.
comfy-table would have to decide several things on its own, for any given arbitrary input:

  • How big should columns be
  • Which columns should get more attention
  • Which columns should be hidden

These are things that cannot be decided automatically. Attempting to do so so will most likely only result in frustrated users.

@arturdaraujo
Copy link

arturdaraujo commented Aug 31, 2023

The 3 parameters you picked for setting a dynamic number of columns could solve this issue (which should not be default because it would make many users frustrated, I agree).

  1. How big? A couple of thoughts. You could set a hard value, like 30 characters (I don't think that's too important since the user would be able to change it). But if you need a default I would get the terminal space and divide by 5, generating 5 columns based on the available space. Columns will be wider if more space is available.

  2. Which columns get more attention?
    I'm not sure what this would mean, In my user cases all columns are treated equally (Polars Dataframes)

  3. For columns hidden, I think there are 2 good options for this.
    3.1. The columns in the middle should be hidden, this means that the order to "add" columns is: 1st column, last column, 2nd column, before last column, 3rd column, and so on until the terminal has no more space available. Once it runs out of space, you place a [...] in the middle column.
    3.2. Pick the first columns until space runs out. The [...] will be the last column in this case.

Maybe @randomgambit @alexander-beedie could give some insight here?

@Techassi
Copy link
Contributor

#126 might solve some of the mentioned issues. However, I'm not sure, if it will be the final version of an improved column handling API.

@Nukesor
Copy link
Owner Author

Nukesor commented Apr 6, 2024

The feature needed by @arturdaraujo is orthogonal to the discussion of new column handling. Based on their requirements, they need a way to cut off columns at some point that shouldn't be displayed.

This can also be done right now to a degree, even though it's a bit cumbersome. The way one would do it right now is to populate the table, get the max widths of each column's content and simply hide all columns that would result in the table being wider than the terminal.

I think it would be nice to achieve this behavior in a more convenient way, so I created #143 to track this feature request.

@Nukesor
Copy link
Owner Author

Nukesor commented Apr 6, 2024

Closed for a clean new discussion in #144

@Nukesor Nukesor closed this as completed Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: discussion Things that need a bit of a discussion t: enhancement Enhancing an existing feature or code
Projects
None yet
Development

No branches or pull requests

3 participants