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

Bug: create_iso8601 doesn't consider the rationality of date/time. #102

Open
Longfei2 opened this issue Oct 28, 2024 · 7 comments
Open

Bug: create_iso8601 doesn't consider the rationality of date/time. #102

Longfei2 opened this issue Oct 28, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@Longfei2
Copy link

What happened?

{FB3DFDF5-4F80-4658-AFCE-4076E6D298F0} please see the screenshot, `2020-02-60` is not a reasonable date. it should output `2020-02`.

Session Information

No response

Reproducible Example

create_iso8601(c("2020-02-60"), .format = "y-m-d")

@Longfei2 Longfei2 added the bug Something isn't working label Oct 28, 2024
@github-project-automation github-project-automation bot moved this to Product Backlog in sdtm.oak R package Oct 28, 2024
@ramiromagno
Copy link
Collaborator

ramiromagno commented Oct 28, 2024

Thanks @Longfei2 for your report.

@rammprasad : I am assigning this issue to you as it was your call not to perform semantic validation on parsed components, see

# No semantic validation is performed on the numeric months, so `"13"` stays
# `"13"` but representations that can't be represented as two-digit numbers
# become `NA`.
expect_identical(
iso8601_mon(c("13", "99", "100", "-1")),
c("13", "99", NA, NA)
)

@botsp
Copy link

botsp commented Oct 29, 2024

It seems unlikely for this to happen in real data because most EDC system itself has edit check for data validity. Data like 2020-02-60 cannot input as raw data.

Another case that the uncontrolled character input(UNK, UN, UK...) can occur is when the collection date is partial. The current function seems to have already considered this issue.
image

@Longfei2
Copy link
Author

Longfei2 commented Oct 29, 2024

However, actually, in my company, there are some date issues, e.g. year may be 0000 in system, Feb has 28 or 29 days depends on whether it is a leap year or not. You should cosider these unreasonable dates。

@botsp
Copy link

botsp commented Oct 29, 2024

Thanks for the sharing;

But those issues should be addressed by the DM side, not through the SDTM mapping process. Relying on this process to "correct" data can lead to risks of losing traceability and cause inconsistencies between SDTM and EDC collection.

Specifically, 2020-02 is not the expect result in SDTM. Adding a function to the current package to achieve that expectation could bring additional compliance risks.

@Longfei2
Copy link
Author

Longfei2 commented Oct 31, 2024

@botsp
Dear author,
Thank you for your answer. In addition to this question, regarding the use of argument ..., I also feel very confused.

In help document, ... is interpreted as "Character vectors of dates, times or date-times' components.". In my view, I can input multiple different vectors as arguments, like this:

sdtm.oak::create_iso8601("2021-02-21", "2022-02", .format = list("y-m-d", "y-m"))

Actually, I expect the output is vector or list like
{5B24D82F-D586-4937-A008-F4B6F16BFECB}

However, this output is only 2021-02-21.

{46D306DB-F118-41CB-9803-1D172EF210F1}

In my view, if you intend to output a charcter vector, you should only chose single argument, like x or datatime, as your argument instead of ..., otherwise, you should return a list for different vectors input.

Of course, it's possible that I didn't understand your development ideas.

I'm glad to have a discussion with you.

@Longfei2
Copy link
Author

Dear,
Sorry to bother you!

I am currently developing an internal package for my company to support SDTM output, so I am very concerned about the status of sdtm.oak and hope that your developed functions can bring some convenience to my work, so I will test some of your functions.

Thank you very much for creating such a great and necessary tool - sdtm.oak.

@Longfei2
Copy link
Author

Longfei2 commented Oct 31, 2024

Dear,

regarding partial date issue, there is unexpected output for m-d format input.
{D9D39C97-F701-46C5-B395-EE322E343698}

According to CDISC SDTM IG - 4.4.2 Date/Time Precision, expected output like:

{9DD39344-2EE9-4447-BDAE-B16CC5C94E55}

These partial date issues actually has some solutions in CDISC SDTM IG - 4.4.2 Date/Time Precision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Product Backlog
Development

No branches or pull requests

4 participants