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

time.clock() should emit a DeprecationWarning #75984

Closed
vstinner opened this issue Oct 17, 2017 · 35 comments
Closed

time.clock() should emit a DeprecationWarning #75984

vstinner opened this issue Oct 17, 2017 · 35 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 31803
Nosy @malemburg, @freddrake, @abalkin, @vstinner, @benjaminp, @benhoyt, @ethanfurman, @serhiy-storchaka
PRs
  • bpo-31803: Remove time.clock() #4018
  • bpo-31803: time.clock() now emits a DeprecationWarning #4020
  • bpo-31803: time.clock becomes an alias to time.perf_counter #4062
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2017-12-19.00:46:51.284>
    created_at = <Date 2017-10-17.13:00:45.598>
    labels = ['3.7', 'library']
    title = 'time.clock() should emit a DeprecationWarning'
    updated_at = <Date 2017-12-19.00:46:51.282>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-12-19.00:46:51.282>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-12-19.00:46:51.284>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2017-10-17.13:00:45.598>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31803
    keywords = ['patch']
    message_count = 35.0
    messages = ['304502', '304503', '304504', '304505', '304506', '304507', '304508', '304513', '304516', '304536', '304537', '304555', '304556', '304561', '304565', '304567', '304569', '304572', '304664', '304668', '304671', '304746', '304747', '304748', '304749', '304885', '304941', '304948', '304997', '304999', '305004', '305040', '305046', '305088', '308598']
    nosy_count = 9.0
    nosy_names = ['lemburg', 'fdrake', 'belopolsky', 'vstinner', 'benjamin.peterson', 'mrabarnett', 'benhoyt', 'ethan.furman', 'serhiy.storchaka']
    pr_nums = ['4018', '4020', '4062']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue31803'
    versions = ['Python 3.7']

    @vstinner
    Copy link
    Member Author

    The behaviour of the time.clock() function is not portable: on Windows it mesures wall-clock, whereas it measures CPU time on Unix. Python has time.process_time() and time.perf_counter(), since Python 3.3, which are portable, well defined and have a better resolution.

    time.clock() was deprecated in Python 3.3 documentation, but calling time.clock() didn't raise a DeprecationWarning. I proposed to remove it from Python 3.7.

    Sometimes, a deprecated function raises a DeprecationWarning in Python version N, before removing it from Python version N. time.clock() doesn't emit such warning, but I consider that it is possible to remove it anyway.

    While it can be annoying to have to patch code to no more use time.clock(), I consider that it's worth it for portability and better clock resolution.

    Attached PR removes time.clock() and time.get_clock_info() doens't accept 'clock' anymore.

    Current time.clock() documentation:
    ----------------------------
    .. function:: clock()

    .. index::
    single: CPU time
    single: processor time
    single: benchmarking

    On Unix, return the current processor time as a floating point number expressed
    in seconds. The precision, and in fact the very definition of the meaning of
    "processor time", depends on that of the C function of the same name.

    On Windows, this function returns wall-clock seconds elapsed since the first
    call to this function, as a floating point number, based on the Win32 function
    :c:func:`QueryPerformanceCounter`. The resolution is typically better than one
    microsecond.

    .. deprecated:: 3.3
    The behaviour of this function depends on the platform: use
    :func:`perf_counter` or :func:`process_time` instead, depending on your
    requirements, to have a well defined behaviour.
    ----------------------------

    @vstinner vstinner added 3.7 (EOL) end of life stdlib Python modules in the Lib dir labels Oct 17, 2017
    @vstinner
    Copy link
    Member Author

    With the PR, Python 3.7 will still requires the C clock() function to build on Unix, since time.process_time() uses it as the last fallback if all other functions failed.

    Maybe we can require to have other functions used by time.process_time() (clock_gettime(CLOCK_PROF), getrusage(), times(), ...), but I consider that it can be done later. This change may impact the Python portability: compilation error when building Python, see bpo-22624.

    @benhoyt
    Copy link
    Mannequin

    benhoyt mannequin commented Oct 17, 2017

    I don't think this is a good idea. I still use time.clock() out of habit (on Windows), and from the PR it's clear that the standard library and tests do too. I wouldn't be at all surprised to find others do as well.

    I realize it's been deprecated since 3.3, but without a DeprecatingWarning, that's easy to miss. What about adding a DeprecatingWarning for 3.7 and deprecate it later, maybe in 3.9? (I know that's a long time, but time.clock() is an old function so we should be cautious!)

    @serhiy-storchaka
    Copy link
    Member

    I think it is better to not remove time.clock() until EOL of 2.7. And in any case it first should start emitting a deprecation warning for a one or two releases.

    @vstinner
    Copy link
    Member Author

    Ben Hoyt: "... from the PR it's clear that the standard library and tests do too."

    I disagree here. The standard library doesn't use time.clock() since Python 3.3. Better clocks like time.perf_counter() or time.monotonic() are now used in the standard library.

    My PR onl changes two old tests and the very old turtledemo. I never used turtledemo, I didn't now that it exists neither :-)

    "I wouldn't be at all surprised to find others do as well."

    Oh, me neither. I'm quite sure that time.clock() is used in the wild. The problem is that you should not use it :-)

    "I realize it's been deprecated since 3.3, but without a DeprecatingWarning, that's easy to miss. What about adding a DeprecatingWarning for 3.7 and deprecate it later, maybe in 3.9? (I know that's a long time, but time.clock() is an old function so we should be cautious!)"

    I never understood the willingness of DeprecationWarning since almost nobody configures Python to run them. In my experience, even when they are displayed, they are usually ignored until the feature is really removed and then people only really start to look at the issue :-)

    But I'm fine with keeping the function and emit a warning in Python 3.7, and remove it from Python 3.8.

    @serhiy-storchaka
    Copy link
    Member

    A runtime deprecation warning was not emitted in 3.3 because two alternatives, time.process_time() and time.perf_counter(), were not available before 3.3. It would be hard to write a warning-free code that supports 3.3 and earlier versions at the same time. But now 3.2 is virtually out of use and I think we can start emitting a deprecation warning at runtime.

    And maybe make 2to3 replacing time.clock() with time.process_time() or time.perf_counter()?

    @vstinner
    Copy link
    Member Author

    Serhiy: "I think it is better to not remove time.clock() until EOL of 2.7. And in any case it first should start emitting a deprecation warning for a one or two releases."

    I really hate using 2.7 EOL as the final countdown to start changing things. Are we building a new "Python 3" chaos where everything explode at once?

    IMHO it's not that hard to write code compatible with Python 2 and Python 3:

    try:
    from time import perf_counter # Python 3
    except ImportError:
    from time import clock as perf_counter # Python 2

    time.clock() is probably not the only code which requires two code paths in any non trivial application which wants to stay compatible with Python 2.

    time.clock() is usually used for benchmarking. I hope that all benchmarking code was already patched to use time.perf_counter() when available, to make the code portable and get better resolution on Unix.

    @mrabarnett
    Copy link
    Mannequin

    mrabarnett mannequin commented Oct 17, 2017

    @victor: True, people often ignore DeprecationWarning anyway, but that's their problem, at least you can say "well, you were warned". They might not have read the documentation on it recently because they have not felt the need to read again about a function with which they are already familiar.

    @vstinner
    Copy link
    Member Author

    I proposed PR 4020 to emit a DeprecationWarning in time.clock() and time.get_clock_info('clock').

    @vstinner
    Copy link
    Member Author

    New changeset 884d13a by Victor Stinner in branch 'master':
    time.clock() now emits a DeprecationWarning (GH-4020)
    884d13a

    @vstinner
    Copy link
    Member Author

    Ok, I modified time.clock() and time.get_clock_info('clock') to emit a DeprecationWarning.

    Let's wait for Python 3.8 to remove time.clock() ;-)

    Thanks for the reviews Serhiy Storchaka and Diana Clarke!

    @vstinner vstinner changed the title Remove not portable time.clock(), replaced by time.perf_counter() and time.process_time() time.clock() should emit a DeprecationWarning Oct 17, 2017
    @malemburg
    Copy link
    Member

    time.cock() is used in a lot of code. Why can't we simply replace the functionality with one of the other functions ?

    The documentation certainly allows for such a change, since it pretty much just says that only the delta between two values has a meaning.

    @ethanfurman
    Copy link
    Member

    I agree with MAL; removing functions just makes multi-version code more painful. At least have the DeprecationWarning active for two releases before removing it.

    @vstinner
    Copy link
    Member Author

    Marc Andre Lemburg: "time.cock() is used in a lot of code. Why can't we simply replace the functionality with one of the other functions ? The documentation certainly allows for such a change, since it pretty much just says that only the delta between two values has a meaning."

    Currently time.clock() is the same clock than time.perf_counter() on Windows, but it's closer to CPU time as time.process_time() on Unix.

    time.perf_counter() and time.process_time() have a different name because the they are different clocks and don't measure the same thing. The main obvious difference is that time.process_time() doesn't include time elapsed during sleep.

    >>> import time
    >>> c1=time.perf_counter(); p1=time.process_time(); time.sleep(1); c2=time.perf_counter(); p2=time.process_time()
    >>> c2-c1, p2-p1
    (1.0010841980038094, 7.308599999999998e-05)

    I don't see how to modify clock() to make it behave the same on Windows and Unix without breaking any application which relies on the current clock() behaviour.

    @vstinner
    Copy link
    Member Author

    (I reopen the issue since the discussion is not over.)

    Marc-Andre Lemburg: "time.cock() is used in a lot of code."

    I ran a quick search on GitHub. I found different use cases of time.clock():

    1. Measure performance. On Windows, time.clock() has a very good precision, *much* better than any other clock. For example, time.process_time() has a resolution of 15.6 ms whereas time.clock() has a resolution of 100 ns (or 0.0001 ms):
      https://www.python.org/dev/peps/pep-0564/#windows

    2. An explicit check that time.clock() doesn't include sleep. I guess that people are using such snippet to explain this behaviour?

    https://github.com/pbarton666/PES_Python_examples_and_solutions/blob/master/py_time.py

    ---

    #py_time.py

    import time
    print(time.clock(), time.time())
    time.sleep(1)  #seconds
    print(time.clock(), time.time())

    Ethan: "I agree with MAL; removing functions just makes multi-version code more painful."

    We have two choices:

    • Deprecate and then remove time.clock(): break the backward compatibility -- currently chosen solution
    • Modify time.clock() to get a portable behaviour: break the backward compatibility

    Depending which clock we choose for time.clock(), we may break more and less code, I would vote for using the time.perf_counter() clock in time.clock(). It means no change on Windows, but modify the behaviour on Unix if the code sleeps (time.sleep or I/O).

    It seems like time.clock() is mostly used for benchmarking, and time.perf_counter() is documented as the best clock for such use case. In the benchmarks I saw on GitHub, the benchmarked code didn't sleep, it was more likely pure CPU-bound.

    Marc-Andre, Ethan: What do you think of removing the deprecation warning from the C (my last commit), leave the deprecation warning in the documentation, and modify time.clock() to become an alias to time.perf_counter()?

    By alias, I really mean time.clock = time.perf_counter, so time.clock.__name__ would say "perf_counter".

    @vstinner vstinner reopened this Oct 18, 2017
    @serhiy-storchaka
    Copy link
    Member

    Initially the time module was a thin wrapper around C and OS time-related functions. It may be confusing that the behavior of time.clock() differs from the behavior of C's clock().

    The documentation for clock() on MSDN suggests to use GetProcessTimes() for the behavior conforming the C standard.

    https://msdn.microsoft.com/en-us/library/4e2ess30.aspx

    @vstinner
    Copy link
    Member Author

    "Initially the time module was a thin wrapper around C and OS time-related functions. It may be confusing that the behavior of time.clock() differs from the behavior of C's clock()."

    Well, there is the theory, and there is the practice. Someone decided to implement time.clock() with QueryPerformanceCounter() on Windows, and so time.clock() is no more a thin wrapper to the C clock() function since at least Python 2.7 (I didn't check earlier versions).

    Portability on clocks is even more than complex than in the os module, especially when you want the same behaviour on Windows and Unix. The PEP-418 added new clocks with a better defined behaviour to "fix" this issue.

    "The documentation for clock() on MSDN suggests to use GetProcessTimes() for the behavior conforming the C standard."

    time.process_time() is implemented with GetProcessTimes(). That's why time.clock() suggests to either replace it with time.perf_counter() or time.process_time().

    @malemburg
    Copy link
    Member

    On 18.10.2017 11:45, STINNER Victor wrote:

    Marc-Andre, Ethan: What do you think of removing the deprecation warning from the C (my last commit), leave the deprecation warning in the documentation, and modify time.clock() to become an alias to time.perf_counter()?

    By alias, I really mean time.clock = time.perf_counter, so time.clock.__name__ would say "perf_counter".

    That's what I think would be a better solution, since the
    absolute value of time.clock() is never used, only the difference.

    If you then get better accuracy in that difference, things
    can only get better, so this is not really backwards compatibility
    issue (nothing gets worse).

    Not sure whether the function name would cause an incompatibility
    issue. I doubt it, but if it does we could have time.clock()
    as function which then simply calls time.perf_counter().

    @vstinner
    Copy link
    Member Author

    I wrote the PR 4062 which removes time.clock() implementation: time.clock becomes an alias to time.perf_counter.

    haypo@selma$ ./python
    Python 3.7.0a2+ (heads/clock-dirty:16b6a3033e, Oct 20 2017, 18:55:58) 
    >>> import time
    >>> time.clock is time.perf_counter
    True

    @serhiy-storchaka
    Copy link
    Member

    This will break a code that depends on the current behavior on non-Windows platforms. And this will contradict the expectation of non-Windows programmers. If change the behavior of clock() on non-Windows platforms, it should be done only after the period of emitting FutureWarning.

    @ethanfurman
    Copy link
    Member

    We definitely need time with either DeprecationWarning or FutureWarning before removing/substituting a function.

    @malemburg
    Copy link
    Member

    On 20.10.2017 19:46, Serhiy Storchaka wrote:

    This will break a code that depends on the current behavior on non-Windows platforms. And this will contradict the expectation of non-Windows programmers. If change the behavior of clock() on non-Windows platforms, it should be done only after the period of emitting FutureWarning.

    Could you explain which behavior is changed by this on non-Windows
    platforms ?

    time.clock() only switches to a more accurate clock. That's pretty
    much it. Which clock it used on which platform was platform
    dependent anyway, so there's no real change in behavior.

    For benchmarking and other measurements, time.time() was recommended
    on Unix and time.clock() on Windows. time.clock() never good
    resolution on Unix, so the situation only improves by using
    a more accurate clock.

    @serhiy-storchaka
    Copy link
    Member

    On non-Windows platforms clock() returns the processor time, perf_counter() does include time elapsed during sleep.

    >>> import time
    >>> start = time.clock(); time.sleep(1); print(time.clock() - start)
    9.700000000001374e-05
    >>> start = time.perf_counter(); time.sleep(1); print(time.perf_counter() - start)
    1.000714950998372

    @malemburg
    Copy link
    Member

    On 22.10.2017 15:14, Serhiy Storchaka wrote:
    > 
    > Serhiy Storchaka <storchaka+cpython@gmail.com> added the comment:
    > 
    > On non-Windows platforms clock() returns the processor time, perf_counter() does include time elapsed during sleep.
    > 
    >>>> import time
    >>>> start = time.clock(); time.sleep(1); print(time.clock() - start)
    > 9.700000000001374e-05
    >>>> start = time.perf_counter(); time.sleep(1); print(time.perf_counter() - start)
    > 1.000714950998372

    Thanks for pointing that out. I didn't know.

    Is there a different clock with similar accuracy we can use
    to only count CPU time on Unix ?

    @serhiy-storchaka
    Copy link
    Member

    Yes, it is time.process_time(). This was the cause of adding two new functions time.perf_counter() and time.process_time() and deprecating (in the documentation only) time.clock(). On Windows time.clock() does include time elapsed during sleep, on non-Windows it doesn't. time.clock() should be replaced with the one of these functions, depending on the purpose of its use. Only the user knows for what purposes it uses time.clock() and what is the correct replacement.

    @vstinner
    Copy link
    Member Author

    Marc-Andre Lemburg: "Thanks for pointing that out. I didn't know."

    Do you still think that we need to modify time.clock() rather than deprecating it?

    @malemburg
    Copy link
    Member

    On 24.10.2017 11:23, STINNER Victor wrote:

    Marc-Andre Lemburg: "Thanks for pointing that out. I didn't know."

    Do you still think that we need to modify time.clock() rather than deprecating it?

    Yes, to avoid yet another Python 2/3 difference. It should be
    replaced with the appropriate variant on Windows
    and non-Windows platforms. From Serhiy's response that's
    time.process_time() on non-Windows platforms and time.perf_counter()
    on Windows.

    The documentation can point to the new functions and recommend
    these over time.clock().

    @vstinner
    Copy link
    Member Author

    Marc-Andre: "Yes, to avoid yet another Python 2/3 difference. It should be replaced with the appropriate variant on Windows and non-Windows platforms. From Serhiy's response that's time.process_time() on non-Windows platforms and time.perf_counter() on Windows."

    I don't understand why you mean by "replaced with". Do you mean modify the implementation of the time.clock()?

    I would like to kill time.clock() beceause it behaves differently on Windows and non-Windows platforms. There are two choices:

    • deprecate time.clock() and later remove time.clock() -- it's deprecated since Python 3.3, and Python 3.7 now emits a DeprecationWarning
    • modify time.clock() to get the same behaviour on all platforms: I proposed to modify time.clock() to become a simple alias to time.perf_counter()

    Now I'm confused. I'm not sure that I understood what you suggest.

    Note: time.clock() already behaves like time.perf_counter() on Windows and time.process_time() on non-Windows. It's exactly how it's implemented. But I consider that it's a bug, and I want to fix it.

    "The documentation can point to the new functions and recommend
    these over time.clock()."

    It's already done in the doc since Python 3.3, no?

    https://docs.python.org/dev/library/time.html#time.clock

    "Deprecated since version 3.3: The behaviour of this function depends on the platform: use perf_counter() or process_time() instead, depending on your requirements, to have a well defined behaviour."

    @malemburg
    Copy link
    Member

    On 25.10.2017 01:31, STINNER Victor wrote:

    Marc-Andre: "Yes, to avoid yet another Python 2/3 difference. It should be replaced with the appropriate variant on Windows and non-Windows platforms. From Serhiy's response that's time.process_time() on non-Windows platforms and time.perf_counter() on Windows."

    I don't understand why you mean by "replaced with". Do you mean modify the implementation of the time.clock()?

    What I meant is that time.clock() is replaced with the higher
    accuracy timers corresponding to the current time.clock()
    implementation on the various platforms in order to retain
    backwards compatibility.

    In other words:

    if sys.platform == 'win32':
        time.clock = time.perf_counter
    else:
        time.clock = time.process_time

    I know that time.clock() behaves differently on different platforms,
    but this fact has been known for a long time and is being used by
    Python code out there for timing purposes.

    @serhiy-storchaka
    Copy link
    Member

    This is the current behavior. Do you suggest to undeprecate time.clock() after 4 releases of deprecation?

    @abalkin
    Copy link
    Member

    abalkin commented Oct 25, 2017

    I would like to add my voice to MAL's objections. I was not aware of time.clock() depreciation before Victor brought this issue to my attention. time.clock() is a very well-known API eponymous to a venerable UNIX system call. Its limitations and platform dependencies are very well-known as well. For someone new to python, seeing time.clock() would probably not require a trip to the library reference, but seeing time.perf_counter() is likely to cause a WTF reaction even from an experienced python developer.

    @vstinner
    Copy link
    Member Author

    Alexander Belopolsky: "I was not aware of time.clock() depreciation before Victor brought this issue to my attention."

    That's why the function now emits a DeprecationWarning :-)

    Alexander Belopolsky: "time.clock() is a very well-known API eponymous to a venerable UNIX system call. Its limitations and platform dependencies are very well-known as well."

    I'm not sure with "platform dependencies are very well-known".

    "clock" is a the most common name to "get the current time". The problem is that the exact clock used by time.clock() depends on the platforms, and that each clock has a different behaviour. That's where the PEP-418 tried to define correctly what are "clocks" when you care of the portability.

    I'm not sure that we can find an agreement. This issue becomes annoying. I don't know what do do. Close the issue? Revert my commit? Ignore the discussion and work on something else? :-(

    @serhiy-storchaka
    Copy link
    Member

    It is very bad, that the function with such attractive name has different meaning on Windows and Unix. I'm sure that virtually all uses of clock() are broken because its behavior on other platform than used by the author of the code. Adding a DeprecationWarning and finally removing it looks right thing to me. But we should keep it while 2.7 and versions older than 3.3 are in use.

    @malemburg
    Copy link
    Member

    On 26.10.2017 13:46, Serhiy Storchaka wrote:

    It is very bad, that the function with such attractive name has different meaning on Windows and Unix. I'm sure that virtually all uses of clock() are broken because its behavior on other platform than used by the author of the code.

    Not really, no. People who write cross-platform code are well
    aware of the differences of time.clock() and people who just
    write for one platform know how the libc function of the same
    function works on their platform.

    Unix: http://man7.org/linux/man-pages/man3/clock.3.html
    Windows: https://msdn.microsoft.com/en-us/library/4e2ess30.aspx

    @vstinner
    Copy link
    Member Author

    I read again the whole issue and I dislike the "time.clock becomes an alias to time.perf_counter" idea. Calling time.clock() now emits a deprecation warning, the issue is fixed, so I close it.

    Many discussions were about removal of the function. IHMO it's out of the scope of this specific issue and should be discussed later, once someone will propose to remove time.clock().

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants