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

Add additional argument seconds argument for clock.tick #4162

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

krobinson395
Copy link

Issue Addressed

#3976

PR Summary

The goal of this PR was to add the additional functionality requested in #3976, for an additional optional argument to be added to the clock.tick method to allow for the user to request the return in seconds rather than milliseconds as is the current default behavior. I have added this additional optional argument, it should not affect any existing code as the default behavior is left unchanged but rather should only add additional options if needed. I have also added an additional set of tests, modelling the original clock.tick tests, to test the behavior when the seconds argument is enabled.

@henserobbie henserobbie deleted the builtin-delta branch March 22, 2024 14:54
@illume
Copy link
Member

illume commented Mar 31, 2024

@krobinson395 catching up on my long backlog... apologies I didn't notice this until now.

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

The appveyor windows builds found some issues. Looking at the logs when you click through the actions you can see these.

src_c/time.c(512): error C2220: warning treated as error - no 'object' file generated
src_c/time.c(512): warning C4113: 'PyObject *(__cdecl *)(PyObject *,PyObject *,PyObject *)' differs in parameter lists from 'PyCFunction'
src_c/time.c(516): warning C4113: 'PyObject *(__cdecl *)(PyObject *,PyObject *,PyObject *)' differs in parameter lists from 'PyCFunction'

Let me know if you want to continue the change?

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.

None yet

2 participants