Skip to content

240329 align papo clinline date inputs #21

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

Open
wants to merge 5 commits into
base: test
Choose a base branch
from

Conversation

gm-ebs-ext
Copy link

No description provided.

@gm-ebs-ext gm-ebs-ext changed the base branch from main to test March 31, 2025 07:46
@gm-ebs-ext gm-ebs-ext marked this pull request as ready for review March 31, 2025 10:52
@gm-ebs-ext gm-ebs-ext requested a review from a team as a code owner March 31, 2025 10:52
@gm-ebs-ext gm-ebs-ext requested a review from ml-ebs-ext March 31, 2025 11:26
@ml-ebs-ext
Copy link
Collaborator

This single-line change is an improvement, but the ICF variable is not the only one that leads to the "blank timeline" problem.
It's conceivable that a thorough review of the codebase could unearth all the places that require tweaks such as the one in this PR.

I personally would favor an alternative approach, which can be summarized as:

  • Map all date+time dataset columns to dates, taking care that columns used as end_date inside by the module are rounded up to the next day.

This would take care of the whole family of errors in a single place.

There is already something similar in this codebase (T_honor_map_to_flag) that maps "Y/N" columns, as specified in the module API, to logical.

This exercise would be easier in any of our other codebases, because they have a second-generation module wrapper, so I think a good prior step would be to move this codebase to CM and TC.

@luffmark luffmark removed the request for review from a team June 25, 2025 10:41
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.

2 participants