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

Assign date or dateTime that contains only year #1511

Closed
wants to merge 2 commits into from

Conversation

mint-thompson
Copy link
Collaborator

Description:
A FHIR date or dateTime may be 4-digit year. The FSH parser will assign this value a NUMBER token type. When assigning to a date or dateTime element, if a type mismatch occurs, and the original value is a number, try to assign the raw value. If it is a valid 4-digit year, it will be assigned successfully. Otherwise, the original error will be thrown and logged.

I don't really like my implementation here: it's very similar code in all four exporters. I didn't manage to come up with a good way to isolate the "retry with another value" code, but I think it would be better if it looked like that.

Testing Instructions:
Tests are added to exporter classes to demonstrate assignment of these values.

Related Issue:
Fixes #1483

A FHIR date or dateTime may be 4-digit year. The FSH parser will assign
this value a NUMBER token type. When assigning to a date or dateTime
element, if a type mismatch occurs, and the original value is a number,
try to assign the raw value. If it is a valid 4-digit year, it will be
assigned successfully. Otherwise, the original error will be thrown and
logged.
Copy link
Collaborator

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! It looks like we handled a similar situation already in all the different exporters, and I trust past us, so I think this is probably the right approach to handle the error.

@cmoesel
Copy link
Member

cmoesel commented Sep 26, 2024

Just to capture some conversations we've had offline:

  • The spec doesn't explicitly say how dates and datetimes should be assigned.
  • The grammar does include a token format for dates and times without string delimiters
  • Most examples in the spec assign using quoted date strings (e.g., valueDateTime = "2012-01-01").
  • This issue (year-only dates parsed as numbers) is avoided if you just assign it using a string date instead.

While this PR does make it so you can assign year-only dates without using quoted strings, it introduces a fair amount of complexity. So the question we need to answer is if the complexity is worth it or if it's better to leave the complexity out of the code and just say you need to use a string date to make it unambiguous.

@mint-thompson
Copy link
Collaborator Author

Closing without merge, as this adds a lot of complexity for something with a fairly simple workaround of "quote the date".

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.

Can't assign DateTime with only year component
3 participants