Skip to content

Use time.monotonic()#15

Merged
cpcloud merged 3 commits intocpcloud:masterfrom
ddelange:use-monotonic
Sep 3, 2020
Merged

Use time.monotonic()#15
cpcloud merged 3 commits intocpcloud:masterfrom
ddelange:use-monotonic

Conversation

@ddelange
Copy link
Copy Markdown
Collaborator

@ddelange ddelange commented Aug 31, 2020

Hi!

Just some small improvements I spotted while inspecting this already beautifully lean lib.
Fixes #10, closes #13

How does your release flow work? Do you merge to default branch, git tag, and upload to Pypi from there? As an alternative to your versioneer setup, you could use setuptools_scm, designed for this specific purpose. See e.g. john-kurkowski/tldextract#187

Description

time.monotonic() relates to the real time spent inside the main thread, and can not be altered externally by e.g.

  • ntp system time synchronisation (frequent, small impact)
  • automatic time zone changes (infrequent, big impact)
  • users manually changing system time (infrequent, big impact)

and is therefore preferred over time.time when measuring relative times (of a program).

Smaller improvements:

  • Remove init with unnecessary instance attribute assignment (time to load the extension itself is now representative due to explicit start() call)
  • Use slots for lower memory consumption of class instance(s)
  • Simplify stop(), reducing the amount of checks, instance attribute lookups and variable assignments (performance nits)
  • Remove, by definition, the need for the assert statement

time.monotonic() relates to the real time spent inside the main thread, and can not be altered externally by e.g.
- ntp system time synchronisation (frequent, small impact)
- automatic time zone changes (infrequent, big impact)
- users manually changing system time (infrequent, big impact)

and is therefore preferred over time.time when measuring relative times (of a program).

Smaller improvements:
- Remove __init__ with unnecessary instance attribute assignment (time to load the extension itself is now representative due to explicit start() call)
- Use __slots__ for lower memory consumption of class instance(s)
- Simplify stop(), reducing the amount of checks, instance attribute lookups and variable assignments
@ddelange
Copy link
Copy Markdown
Collaborator Author

ddelange commented Sep 3, 2020

@cpcloud as the last release was 2016, what are the chances of this getting reviewed, merged & released by you? i saw some forks arose on pypi, maybe due to inactivity here?

if you're overloaded, do you need help maintaining this lib? it is still used a lot

Comment thread autotime/__init__.py
@cpcloud
Copy link
Copy Markdown
Owner

cpcloud commented Sep 3, 2020

@ddelange I'm definitely interested in having some help maintaining this library. Are you interested?

@ddelange
Copy link
Copy Markdown
Collaborator Author

ddelange commented Sep 3, 2020

definitely would be down :) do you do uploads to pypi manually? if you can give me pypi write access, I could also add Github Actions into the mix to automatically deploy to pypi when a new release/tag is pushed to the repo on master branch (not sure if overkill for this repo tho, can keep doing it manually of course)

@cpcloud
Copy link
Copy Markdown
Owner

cpcloud commented Sep 3, 2020

Alright, I'll add you as a collaborator on the repo along with PyPI perms, doing it now.

@cpcloud
Copy link
Copy Markdown
Owner

cpcloud commented Sep 3, 2020

Actually @ddelange I am apparently not the person who added the PyPI package!

Copy link
Copy Markdown
Owner

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

LGTM, merging.

@cpcloud cpcloud merged commit 4913b5b into cpcloud:master Sep 3, 2020
@ddelange
Copy link
Copy Markdown
Collaborator Author

ddelange commented Sep 3, 2020

@amitu could you add us as maintainers on the pypi package so we can release a new version?

@ddelange
Copy link
Copy Markdown
Collaborator Author

ddelange commented Sep 7, 2020

@cpcloud I've sent @amitu an email on his personal gmail in an attempt to reach him.

If we can't reach him for 6 weeks, we can open an issue with a PEP 541 request: https://www.python.org/dev/peps/pep-0541/#reachability

In the meantime, I didn't find a PyPi account for cpcloud. Do you have one/can you create one?

@ddelange
Copy link
Copy Markdown
Collaborator Author

@amitu just another reminder, please grant us pypi maintainer access so we can release this :)

@amitu
Copy link
Copy Markdown
Contributor

amitu commented Sep 25, 2020

@ddelange apologies for delayed reply. Who do I give access to?

@ddelange
Copy link
Copy Markdown
Collaborator Author

Hi @amitu! No worries. As @cpcloud doesn't have a pypi account (yet?), can you add just ddelange for now?

@cpcloud
Copy link
Copy Markdown
Owner

cpcloud commented Sep 26, 2020

My pypi account is Charles.Cloud

@amitu
Copy link
Copy Markdown
Contributor

amitu commented Sep 28, 2020

I have invited both of you as maintainers. Let me know if someone wants to take over the ownership.

@ddelange
Copy link
Copy Markdown
Collaborator Author

Thanks @amitu :)

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.

Non monotonic time

3 participants