Changed tz_offset to seconds, updated docstring

adafruit / Adafruit_CircuitPython_NTP

Network Time Protocol (NTP) Helper for CircuitPython

MIT License

9 stars 18 forks source link

Changed tz_offset to seconds, updated docstring #11

Closed theelectricmayhem closed 4 years ago

theelectricmayhem commented 4 years ago

@brentru - Apparently I'm not formatting the docstring correctly. Where can I find info on what the formatting rules are?

(sorry for all the hassle for such a small change)

brentru commented 4 years ago

@theelectricmayhem Looks like it's failing on Sphinx (https://github.com/adafruit/Adafruit_CircuitPython_NTP/pull/11/checks?check_run_id=668814279#step:14:1). Try adding a blank line at the end of your docstring:

        positive in the US, zero in the UK).

        """

We have some instructions about running sphinx locally here: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/sharing-our-docs-on-readthedocs#sphinx-5-1,

ladyada commented 4 years ago

@brentru please do the full review :)

theelectricmayhem commented 4 years ago

@brentru I modified it to seconds (from hours) based on feedback from @jepler. Also, it seemed to make the most sense for making adjustments to, say time.localtime() since it's in seconds.

I saw the error about "Field list ends without a blank line; unexpected unindent." but the previous version also did not have a trailing blank line and it did not fail the Sphinx test.

Let me know if you'd prefer minutes rather than seconds and I'll make changes.

theelectricmayhem commented 4 years ago

Mercy! @brentru if you could take a look at this, I'd appreciate it. I can't, for the life of me find the offending formatting.

brentru commented 4 years ago

@theelectricmayhem I'm not sure where the issue is, @sommersoft could you please take a look? Thanks!

jepler commented 4 years ago

The documentation building process is finished. I think the remaining question is: minutes or seconds.

UNIX seems inconsistent on the subject.

Reasons to use minutes:

It suffices for the UTC offset of all time zones in existence at this time
It is used in struct timezone (though the zone argument of gettimeofday/settimeofday is largely unused)

Reasons to use seconds:

Some historical timezones have less-than-minute offsets
Some modern systems move towards using TAI or GPS time as the system time, and represent leap seconds by accounting for them in the local time zone offset (https://www.ucolick.org/~sla/leapsecs/right+gps.html)
Python uses it in time.timezone, time.altzone

Many newer systems store time zone offsets as general time intervals (similar to datetime.timedelta in CPython3) so they can represent arbitrary durations without the problem of knowing what the basic unit actually is.

brentru commented 4 years ago

@jepler I think using seconds would be useful for this library, for now. We could move to time deltas in a future PR (I didn't know about them, thanks for mentioning). Are you OK with this?