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

epinow()'s "timing" output should include units #687

Closed
jamesmbaazam opened this issue Jun 5, 2024 · 2 comments · Fixed by #688
Closed

epinow()'s "timing" output should include units #687

jamesmbaazam opened this issue Jun 5, 2024 · 2 comments · Fixed by #688

Comments

@jamesmbaazam
Copy link
Contributor

Is your feature request related to a problem? Please describe:

I'm using the epinow() function to run and compare the run times of various model specifications to address #629. I am particularly interested in the "timing" output of epinow() but it currently prints a number without units, so it is not interpretable.

Describe the solution you'd like:

The "timing" output of epinow() should include units. The following line

out$timing <- round(as.numeric(end_time - start_time), 1)

can be changed to sub("Time difference of ", "", capture.output(difftime(end_time, start_time))) to fix the issue.

Additional context:
Add any other context or screenshots about the feature request here.ocument Title

@sbfnk
Copy link
Contributor

sbfnk commented Jun 6, 2024

Good point - however, would it not make more sense to just store the results of difftime (of class <difftime>? This would then leave the user with the option of printing (getting the nicely formatted output) or processing numerically.

@jamesmbaazam
Copy link
Contributor Author

This would then leave the user with the option of printing (getting the nicely formatted output) or processing numerically.

That's a good point. I've made the changes in the linked PR.

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 a pull request may close this issue.

2 participants