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

Sort by time.Date #90

Closed
copejon opened this issue Jan 1, 2020 · 12 comments · Fixed by #100
Closed

Sort by time.Date #90

copejon opened this issue Jan 1, 2020 · 12 comments · Fixed by #100
Assignees
Labels
disc General discussion enhancement New feature or request

Comments

@copejon
Copy link

copejon commented Jan 1, 2020

Firstly, thank you for this thoughtfully implemented project! The documentation (via comments and code) are very well done and the use of library is very intuitive!

Is your feature request related to a problem? Please describe.
As the title hints, sorting by the builtin time.Time struct would be a logical additional feature to this project. I have a personal project used to translate and reconcile a my bank account against my budget app (don't ask me why they don't already). So naturally, I want the table to be printed chronologically.

My current solution is to print an additional column for the unix (int) representation of the time and sort the table by that column. This works fine but for a table hundreds or thousands of rows long, it's a pretty ugly output. I'm functionally printing 2 time.Time columns, in different formats.

Describe the solution you'd like
An option to sort tables by time.Time values.

Describe alternatives you've considered
Defining an additional column to list unix time, then sorting that by integer values. It's ugly and requires extra, likely duplicated code. Pretty much anyone who wants to sort by date will need two columns - one for human readable time, and one for sortable time.

Additional context
Add any other context or screenshots about the feature request here.

My project is still incomplete, but if you're interested to see how i'm consuming this project: https://github.com/copejon/reconcilitator

Excuse the multitude of readme typos. Written late at night and not as fun to fix as coding.

@jedib0t
Copy link
Owner

jedib0t commented Jan 1, 2020

Hey. Thanks for the kind words about the code. 👍

I think sorting should work transparently on a column that contains only time.Time values. I'll double check anyhow and let you know.

@jedib0t jedib0t self-assigned this Jan 2, 2020
@jedib0t
Copy link
Owner

jedib0t commented Jan 2, 2020

Hey @copejon ... I read your code where you were using go-pretty/table. I see that you are using const layout = "01/02/2006" when using text.NewTimeTransformer(layout, nil).

Is it a must that your date/time be in that format? I usually find date/time very readable (and also lends itself automatically for sorting) if you use a layout like time.RFC3339 ("2006-01-02T15:04:05Z07:00").

For example, the code below:

package main

import (
	"os"
	"time"

	"github.com/jedib0t/go-pretty/table"
	"github.com/jedib0t/go-pretty/text"
)

func main() {
	now := time.Now()

	tw := table.NewWriter()
	tw.AppendHeader(table.Row{"Index", "Time"})
	tw.AppendRow(table.Row{1, now.Add(time.Hour)})
	tw.AppendRow(table.Row{2, now.Add(time.Hour * 2)})
	tw.AppendRow(table.Row{3, now})
	tw.AppendRow(table.Row{4, now.Add(time.Minute * 30)})
	tw.AppendRow(table.Row{5, now.Add(time.Second * 50)})
	tw.AppendRow(table.Row{6, now})
	tw.SetColumnConfigs([]table.ColumnConfig{{
		Name:        "Time",
		Transformer: text.NewTimeTransformer(time.RFC3339, nil), // "2006-01-02T15:04:05Z07:00"
	}})
	tw.SortBy([]table.SortBy{
		{Name: "Time", Mode: table.Asc},
		{Name: "Index", Mode: table.AscNumeric},
	})
	tw.SetOutputMirror(os.Stdout)
	tw.Render()
}

produces the following output:

+-------+---------------------------+
| INDEX | TIME                      |
+-------+---------------------------+
|     3 | 2020-01-01T19:39:08-08:00 |
|     6 | 2020-01-01T19:39:08-08:00 |
|     5 | 2020-01-01T19:39:58-08:00 |
|     4 | 2020-01-01T20:09:08-08:00 |
|     1 | 2020-01-01T20:39:08-08:00 |
|     2 | 2020-01-01T21:39:08-08:00 |
+-------+---------------------------+

Note:

  • If you don't care for the time, and just want the date, you can use the layout "2006-01-02" instead and get the same functionality, but the sorting would not be accurate when multiple rows have the same "date" (but different times)
  • If you use layout "2006-01-02 15:04:05", it will be readable and sort with seconds-level precision
  • If you use layout "2006-01-02 15:04:05.000", it will be readable and sort with microseconds-level precision (see sample output below):
+-------+-------------------------+
| INDEX | TIME                    |
+-------+-------------------------+
|     3 | 2020-01-01 19:44:29.377 |
|     6 | 2020-01-01 19:44:29.377 |
|     5 | 2020-01-01 19:45:19.377 |
|     4 | 2020-01-01 20:14:29.377 |
|     1 | 2020-01-01 20:44:29.377 |
|     2 | 2020-01-01 21:44:29.377 |
+-------+-------------------------+

Basically go-pretty/table cheats and just does text sorting most of the time since the final output is pretty much just a text string regardless of the underlying data-type. The modes table.Asc and table.Dsc just tell the code which way to sort. When you specify table.AscNumeric or table.DscNumeric, it tries to convert the final string back into a number and then sorts the same.

@jedib0t jedib0t added the disc General discussion label Jan 2, 2020
@jedib0t
Copy link
Owner

jedib0t commented Jan 2, 2020

If you still want to keep the layout, let me know and I'll try to figure out a way to support sorting the contents before transforming it to text.

@copejon
Copy link
Author

copejon commented Jan 4, 2020

@jedib0t Thanks for the detailed response!

Is it a must that your date/time be in that format? I usually find date/time very readable (and also lends itself automatically for sorting) if you use a layout like time.RFC3339

The layout was chosen because it matches that used by the budget app and the bank in their transactions. This makes it easier to cross reference flagged transactions from my app's output against the bank and budget registers:

e.g.

+------------+-------------------------------------+----------+---------+------------+
| DATE       | PAYEE                               |   AMOUNT | CLEARED |       SORT |
+------------+-------------------------------------+----------+---------+------------+
| 12/24/2019 | Apple                               |   -10.81 | true    | 1577145600 |

However, that may not a very compelling reason to add the feature and I can see why it makes sense to implement it in the way you have. Expanding the behavior to specifically target time.Time is probably out of scope.

Another thought that might be more inline with the project could be a ColumnPainter, with an option to hide a column. No worries if that doesn't sound like a good fit though. Thanks again for feedback!

@copejon
Copy link
Author

copejon commented Jan 4, 2020

Of course, there's also the point that I should probably organize my data a little bit better :)

@jedib0t
Copy link
Owner

jedib0t commented Jan 4, 2020

It does make sense to have a sorting function before converting to text. I get your use-case. Give me a week or so and I'll get something working for you.

@jedib0t
Copy link
Owner

jedib0t commented Jan 28, 2020

Hey @copejon ... sorry for no updates these past 25 days. I've not forgotten about this, and will try to find some time to add a pre-render sort feature.

@copejon
Copy link
Author

copejon commented Jan 30, 2020

@jedib0t No worries! I've been pretty distracted with actual work projects, so it's been on my back burner as well.

@jedib0t
Copy link
Owner

jedib0t commented May 7, 2020

Hey @copejon on further thought, your idea of an option to hide a column makes a lot of sense. I'll implement an option in ColumnConfig and get it out this week. Sorry for the obscene delays.

@copejon
Copy link
Author

copejon commented May 7, 2020

Don't sweat it! My project hasn't been impacted - just thought it would be a nice-to-have :) Glad I could contribute the idea.

jedib0t added a commit that referenced this issue May 8, 2020
table: support hiding columns on render; fixes #90
@jedib0t
Copy link
Owner

jedib0t commented May 8, 2020

Hey @copejon ... sorry for not giving you a proper pre-process-sort option, but with the new Hidden flag in ColumnConfig, you should be able to sort by a field, but mark it as hidden.

Please try it and let me know if it works or if you find any bugs.

Appreciate your patience! ✋

@copejon
Copy link
Author

copejon commented May 8, 2020

+1 thanks!

@jedib0t jedib0t added the enhancement New feature or request label May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disc General discussion enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants