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

Improve event labels #151

Open
wants to merge 31 commits into
base: dev
Choose a base branch
from
Open

Improve event labels #151

wants to merge 31 commits into from

Conversation

LDSamson
Copy link
Collaborator

@LDSamson LDSamson commented Dec 19, 2024

Closes #67. This PR adds a lot of flexibility in creating event/visit labels.

The event tab in the metadata now looks like this: image

The new setup enables the user to set the logic on which the event labels are created, and to set the names (based on prefix and suffix).

In addition, if a variable with the name 'event_name_edc' is available in the data, and no suffix is set in the table, the event_name_edc will be added to the visit names:
image
in the timeline:
image

Lastly, the labels in the compact timeline top right are now the number of 'expected events' but this expand flexibly if more visits occur.
image
The only thin less flexible are the labels for this compact timeline; these are now always V0-Vx. I don't know how you guys think of this.

The metadata needs to be updated to be able to benefit from this update.

@LDSamson LDSamson marked this pull request as draft December 20, 2024 08:02
@LDSamson LDSamson force-pushed the ls_improve_event_labels branch from 4527e13 to 2988859 Compare December 20, 2024 11:55
@LDSamson LDSamson force-pushed the ls_improve_event_labels branch from 2988859 to 5315685 Compare December 20, 2024 11:58
@LDSamson LDSamson force-pushed the ls_improve_event_labels branch from 78191a0 to fda87ad Compare December 20, 2024 12:44
@LDSamson LDSamson marked this pull request as ready for review December 20, 2024 12:54
@jthompson-arcus
Copy link
Collaborator

@LDSamson I will give it a look. First impressions is that this PR actually reduces flexibility for us. We built the events tab off of the visits, so instead of using V0-Vx (or Visit 1 - Visit X), we were using C1D1-CXDY and there applicable names. This is also why we wanted the additional flexibility during merge_meta_with_data(). Further hard-coding the V0-Vx notation in the app wouldn't be a direction we would want to go. Our events tab looks like this:

event_number event_name event_label
1 Screening SCR
2 Cycle 1 Day 1 C1D1
6 Cycle 1 Day 3 C1D3
20 Cycle 2 Day 1 C2D1

@LDSamson
Copy link
Collaborator Author

I was not aware that you customized the events tab. This was previously only used for the compact timeline. On a side note, are you using the event_number for something specific? I don't think that was used in clinsight.

'Hard coding' was done for the small timeline but this can be changed. I think the rest is much more flexible in this way, also the normal 'Visit ...' notation is now completely customizable. You can create the same labels as you have now by filling in the table like this, if these patterns match the event_id in the raw data:

image

But you can still provide your own event_name (or other variables) if you prefer to do so, that did not change.

One advantage of this PR is that there can be many more visits, for example during follow-up. The number of visits do not not necessarily have to be hard coded in advance. And it provides a way to deal with unscheduled visits.

@LDSamson LDSamson linked an issue Jan 14, 2025 that may be closed by this pull request
@LDSamson LDSamson marked this pull request as draft January 14, 2025 10:06
@LDSamson
Copy link
Collaborator Author

Marking as draft until custom visit labels can be used in the compact timeline again

@LDSamson LDSamson marked this pull request as ready for review January 16, 2025 15:01
@LDSamson
Copy link
Collaborator Author

LDSamson commented Jan 16, 2025

Okay, I made improvements and restored the option to customize the compact timeline labels. @aclark02-arcus @jthompson-arcus can you have a look if it works for you?

The events tab in the metadata now looks like this:

grafik

Green columns are optional. From the orange ones, they can be used both but at least one needs to be available.
You can now just fill in the event_id column (previously event_label) and nothing else, and everything should be as expected.

Few more details;

  • Only visits of which is_expected_visit is TRUE will show up in the timeline. Especially useful for 'unscheduled visits' which can happen at any time and should not show up here.
  • Events will be created based on matching with event_id in the study data.
  • The event_id will be taken as label for the compact timeline, but this can be overwritten by event_label_custom
  • If event_id_pattern is used to match events, multiple events can match, and the labels will default to V0-V1-...-Vx for these events.
  • The longer event_name will also default to event_id but can also be customized using event_name_custom. A sequence number will be added to the custom name if needed (add_sequence_to_name). If the column event_name_edc is available in the data, this value will be added as a suffix in brackets:
    grafik

We need this construction mostly because we have studies with an indeterminate number of 'normal' visits and thus need some flexibility in creating event labels.

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.

Need to Generalize add_timevars_to_data()
2 participants