Skip to content

Conversation

@chschan
Copy link
Contributor

@chschan chschan commented Jun 11, 2025

  • Change default y.zero to FALSE (this means that 0 is not forced on the y-axis) and matches behaviour of currrent spline plots
  • Specify format of POSIXct vector being converted to string (as rownames of plot.data), to avoid problems with parsing dates

@chschan chschan requested a review from JustinCCYap June 11, 2025 02:44
# Create plot
plot.data <- pred$fit
names(plot.data) <- pred$predictor
names(plot.data) <- if (inherits(pred$predictor, "POSIXct")) strftime(pred$predictor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should a format be specified for strftime to match what it used to do? Right now does it use the default which is "%Y-%m-%d %H:%M:%S"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

strftime uses the correct default. I think previously it was calling as.character which used a different format, but for `strftime the default format is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the format will change for users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The change affects whether the plotly can correctly parse the dates. If the parsing works, plotly selects nice date defaults (which are the same as before)
image

Copy link
Contributor

@JustinCCYap JustinCCYap left a comment

Choose a reason for hiding this comment

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

LGTM

@chschan chschan merged commit 2ed2e5d into master Jun 11, 2025
3 of 4 checks passed
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