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

ISO8601 ([T])TIME #17

Open
ciozi137 opened this issue Jan 3, 2025 · 5 comments
Open

ISO8601 ([T])TIME #17

ciozi137 opened this issue Jan 3, 2025 · 5 comments

Comments

@ciozi137
Copy link
Collaborator

ciozi137 commented Jan 3, 2025

The library parses a 'strict' ISO8601 string with DATE in many formats, DATETIME with [T] separator, and TIME with a leading [T]. It does not recognize TIME without leading [T] is as strict ISO8601. However, the ISO8601 spec indicates that the leading [T] is not necessary when the string is unambiguously a time.

I propose the following functionality:

  1. ISO8601 String to Timestamp.vi (for Strict?==True) will require a [T] separator unless it can determine if the input string is unambiguously a DATE or a TIME.
    Note: it currently behaves this way except for TIME only.
  2. ISO8601 TimeString to Timestamp.vi should not require leading [T] separator as the method itself removes any ambiguity.
  3. (A hypothetical method ISO8601 DateString to Timestamp.vi would not require a trailing [T] because the input string would unambiguously be a DATE.) 1

This issue will continue discussion found in Issue #11

Footnotes

  1. Perhaps this is a desirable method. ISO8601 String to Timestamp.vi could call both ISO8601 DateString to Timestamp.vi and ISO8601 TimeString to Timestamp.vi and look for a [T] to determine validity?

@ciozi137
Copy link
Collaborator Author

ciozi137 commented Jan 3, 2025

Trying with this structure:
image

If 1996-12-19, 09:20:02, and T09:20:02 are all valid ISO, is 1996-12-19T?

@ciozi137
Copy link
Collaborator Author

ciozi137 commented Jan 3, 2025

@francois-normandin I think this part of the code is mostly working. I will submit pull request with my changes.
image

Notes about existing tests:

1. Strictly Invalid

These failing tests should be fixed and probably can be resolved with logic inside ISO8601 *String to Timestamp.vi:

image

2. Support for Ambiguous

This requires a discussion about what is/is not allowed in v1.3 release and whether the tests need to be changed or not.

image

3. RFC3339

This one I don't understand:

image

ciozi137 added a commit to ciozi137/Epoch-Time that referenced this issue Jan 3, 2025
… strings

- include updated unit tests inside Test Strict Ambiguous ISO8601 Strings.vi
- some strictly invalid tests are failing
@ciozi137 ciozi137 mentioned this issue Jan 3, 2025
@francois-normandin
Copy link
Member

francois-normandin commented Jan 3, 2025

@ciozi137 I'm not sure where you get the failures in the tests you report above. In the current main branch, those are ALL passing.

From this point, let's submit only changes that resolve tests that are breaking and do not break existing tests that passed.
If we have concerns/discussions about the test vectors, let's use this issue to track them and agree on them.

I would like that we submit changes to the test vectors before we resolve to fix potential issues, otherwise, we'll be working on false vectors are optimize the wrong outcome.

@francois-normandin
Copy link
Member

About RFC3339, yes this is confusing.
In Epoch 1.2.0, RFC 3339 == Non-strict ISO8601
In epoch 1.3.0, this paradigm changes because we will correctly support date-time in the Weekday or Ordinal Date formats.

Let's track this in here

@ciozi137
Copy link
Collaborator Author

ciozi137 commented Jan 3, 2025

@francois-normandin failures were in #18
Totally agree let's settle on tests first

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

No branches or pull requests

2 participants