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

Make seconds optional in Local time #671

Closed
peterritter opened this issue Sep 23, 2019 · 18 comments · Fixed by #894
Closed

Make seconds optional in Local time #671

peterritter opened this issue Sep 23, 2019 · 18 comments · Fixed by #894

Comments

@peterritter
Copy link

peterritter commented Sep 23, 2019

Lets face it, the vast majority of daily events start on the minute or on the hour, but not on the second. Since TOML supports 'time of day' then this should really be made more pragmatic and allow the format that pretty much everyone uses for daily events which is HH:MM like:
'09:30' '14:00'
as opposed to
'09:30:00' and '14:00:00'

Obviously the latter HH:MM:SS and HH.MM.SS.F should continue to be supported, but I am essentially asking for HH:MM to be added as a valid time-of-day format.
//---------------------------------------------
EDIT: To keep it simple I am hereby paring down my request to just making the seconds optional (default to :00) in the 'time-of-day' format, since that would already be a big step forward, and I think most would agree that this is a step in the right direction.
//----------------------------------------------

One debatable point is whether '9:30' should also be allowed in addition to '09:30'. I don't see why not. Maybe in future even '9am' and '2pm'. As a human readeable configuration format why would TOML not allow time to be written the way most people actually write it? This wouldn't introduce any ambiguity and updating the parsers would be straight forward too.

Thank you for your consideration!

BTW: I love TOML! Its a great idea! In the past I rolled my own parsers for .ini style files but now this is one less thing I need to worry about :-) Great Job!

@eksortso
Copy link
Contributor

Date and time formats were what got me into TOML in the first place. And there have always been a lot of formats to choose from. See #263, for instance.

But throughout it all, we've stuck to as few format types as possible. Introducing new ones have always come at a cost. We'd be less minimal, but more obvious. In this case, the reason for making seconds optional has to outweigh the need to stick to RFC 3339 as the guiding standard.

You may want to pare your argument down to just making seconds optional in times (defaulting to zero). I'd be okay with just that one change, since it's a common practice and it's legal under ISO 8601.

Others may think differently though.

@ChristianSi
Copy link
Contributor

I'm very much in favor of making seconds optional, as in very many use cases they'll be ":00" anyway. If we do that, we should do it in all time-related types, of course:

OffsetDateTime = 1979-05-27T07:32Z
LocalDateTime = 1979-05-27T07:32
LocalTime = 07:32

I'm skeptical about the single-digit hour. If people can write 7:32, they'll expect to be able to write 1979-05-27 7:32 (local date-time), 1979-05-27T7:32Z (offset date-time) etc. But that's unusual and not optimal for readability, especially if "T" is used as separator.

@mmakaay
Copy link

mmakaay commented Sep 24, 2019

I don't mind putting the extra :00 in a time, to make my (date/)time standards-compliant. So not a feature that I personally miss.

I wouldn't mind supporting :00 as a default for an HH:MM time spec. I wouldn't mind supporting an AM/PM feature for time definition either.

I wouldn't like this at all for the combined date/time definition, with the "T" separator. That notation is way too standard to be tinkering with its formatting options.

When I need a more loose form of date/time definition, I normally look at the date/time parsing that is available in the language at hand, and simply define the date/time as a string that I can then parse from the code. This way, I can get 7am working, but also more complex definitions like 'third Thursday of this month at 7:00`.

@workingjubilee
Copy link
Contributor

workingjubilee commented Sep 24, 2019

Sometimes I think I am not human, because people will say "well, this is more human-readable" about things like AM/PM, whereas I find the entire affair abhorrently difficult to reason around and do in fact use 24 hour time when I can get away with it. AM/PM complicates parsing and maneuvering through times a LOT for no gain -- forcing people to normalize to 24-hour time is usually more advantageous.

I also dislike divergences from a well-accepted standard format that is being "imported" to TOML, because currently you can reuse your existing RFC 3339 code, and adding in an additional :00 here and there to preserve that advantage doesn't seem to hurt things that much. It makes more sense to just say, "you can write it down as a string and then parse it to datetime if you need it that much", much like three is not held as an Integer that is equivalent to 3 even though they are logically the same.

So to the extent that it doesn't "break compatibility" with a valid RFC 3339 parser, I think it makes sense, but might be superfluous, but to the extent that it does, I am wholly against.

@ChristianSi
Copy link
Contributor

ChristianSi commented Sep 25, 2019

@workingjubilee: I agree (possibly because I'm not a native speaker) that the AM/PM stuff is quite non-intuitive and confusing. For example, is 12:05 AM 10 minutes after 11:55 AM, or if not, why not? How are 00:00 (midnight) and 12:00 (noon) written? etc. TOML was designed for the people of the world, not just for native English speakers, hence it shouldn't force everyone to understand these oddities.

But I disagree about the seconds thing. 15:30 and 15:30:00 are both easily understandable and it's not hard to grasp that both mean the same time. RFC 3339 was designed for computers (and it doesn't allow stand-alone 15:30:00 either), while TOML was designed first and foremost for humans. That's a difference we should take into account.

@workingjubilee
Copy link
Contributor

workingjubilee commented Sep 25, 2019

Hmm... I just don't know that we're going to be running in to a ton of cases where the difference matters but I will relent the seconds is of least concern. I do retain a little bit of worry that forcing people to specify a second is actually quite relevant when working with computing, and so in this case it might be wise to impose a little on users.

That said, anyone got copies of r/ISO 8601-[1-2]{1}:2019/? I feel like that would be relevant for this discussion. RFC3339 is based on a now-withdrawn version of ISO 8601, and while I don't think ISO 8601:2004 is now irrelevant, it might be nice to update only once on this.

@peterritter
Copy link
Author

peterritter commented Sep 29, 2019

Thank you for considering my request for making the 'time of day' format simpler.
I have personally implemented comprehensive date and date_time classes for a financial library, so I am very familiar with parsing ISO standard date-time values. However, I personally think of 'time of day' as a completely different animal. date-time is primarily used for time-points or timestamps in applications but 'time of day' is predominantly used as 'user input'.
When I look at my google calendar, the format they chose is '10:30am' , '8:30pm'. To me this is the ideal format as my brain immediately recognizes it as a 'time of day' . When I see '10:30:00' and '08:30:00' the brain needs to think twice - is it some sort of new network address scheme ? It doesn't 'click' as a 'time of day'.
Nonetheless, I have pared down my request to just making the seconds optional (default :00). That would already improve things a lot, and can probably be agreed upon quickly. Thank you for your consideration!

@pradyunsg
Copy link
Member

Marking this as post-1.0 since this is new syntax.

@eksortso
Copy link
Contributor

eksortso commented Nov 6, 2019

Marking this as post-1.0 since this is new syntax.

I wish that you would reconsider. It is new syntax. But it doesn't break the old syntax, and would prove beneficial for version 1.0's adoption.

@ChristianSi
Copy link
Contributor

ChristianSi commented Nov 6, 2019

I agree with @eksortso . We should keep in mind that TOML is meant to be easily read- and writeable. RFC 3339 timestamps, on the other hand, were designed for computers.

@h-vetinari
Copy link

I really wish this would make it into 1.0

@eksortso
Copy link
Contributor

A PR for this would be simple to work up and apply. And right now, with me having no computer to work on, simple is a plus.

@h-vetinari
Copy link

Ping @eksortso :)

@ChristianSi
Copy link
Contributor

@h-vetinari Not sure whether this was @eksortso's intent, but maybe you could prepare a little PR yourself? That would probably help moving this thing forward.

@h-vetinari
Copy link

@ChristianSi
That's a fair thing to ask, but contrary to how I understand @eksortso ("A PR for this would be simple to work up"), I wouldn't know where to start. I'm just someone who's using TOML and would like this feature. :)

But with a pointer to where I'd need to change things, I could maybe try.

@eksortso
Copy link
Contributor

eksortso commented Jan 29, 2020

@h-vetinari @ChristianSi Thanks for bringing this up again. I did want to try writing a PR, but I still don't have a permanent computer to call my own. But you'll hear from me again within 12 hours.

From what I've seen, the ABNF just needs a one-line change, and the README needs updates stating that seconds are optional around the two places where it talks about the precision of fractional seconds.

@eksortso
Copy link
Contributor

Keep in mind that this is still considered post-1.0, though I'd be happy to see it happen sooner than later. Current v0.5 parsers might choke on missing seconds in timestamps. Just putting this PR in perspective.

@pradyunsg pradyunsg changed the title local time of day format should support '09:30' as opposed to '09:30:00' Make seconds options in Local time Mar 6, 2022
@pradyunsg pradyunsg changed the title Make seconds options in Local time Make seconds optional in Local time Mar 6, 2022
@eksortso
Copy link
Contributor

eksortso commented May 3, 2022

I have replaced the older PR #696 with #894. The language changes are identical. But since the older PR was written pre-v1.0.0, it was just easier to rewrite that PR.

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