Skip to content

Conversation

@roninsightrx
Copy link
Contributor

@roninsightrx roninsightrx commented Jul 12, 2025

The issue was just order of things: in the function I split off (parse_input_data), the obs_type_label needs to be handled before the colnames are all converted to lower-case.

I merged this off of tag v1.74, so should just retag 1.74 tag to the SHA for this commit, no need to merge this PR (fix is already in master).


# Check column names are lowercase
expect_equal(names(result), c("t", "obs_type", "y"))
expect_equal(names(result), c("t", "obs_type", "y", "obs_type"))
Copy link
Contributor Author

@roninsightrx roninsightrx Jul 12, 2025

Choose a reason for hiding this comment

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

this is a bit ugly to end up with two columns with same name (if using OBS_TYPE as label), but this was how it was working before, we just didn't have a test on it. Don't want to change more than needed in this PR, we can tackle later.

# Check sorting
expect_equal(result$t, c(1, 2, 3))
expect_equal(result$obs_type, c(1, 1, 1))
expect_equal(result$obs_type, c(1, 2, 1))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was wrong when I set up the test, didn't notice then.

@roninsightrx roninsightrx requested a review from jasmineirx July 12, 2025 07:23
Copy link
Contributor

@jasmineirx jasmineirx left a comment

Choose a reason for hiding this comment

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

lgtm!

@roninsightrx roninsightrx merged commit 06aea9b into master Jul 14, 2025
1 check passed
@roninsightrx roninsightrx deleted the fix-1.74 branch July 14, 2025 19:28
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.

3 participants