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 new module python-stdlib/datetime #449

Closed
wants to merge 39 commits into from

Conversation

lorcap
Copy link
Contributor

@lorcap lorcap commented Sep 26, 2021

This new module is a port of Python datetime providing classes for manipulating dates, times, and deltas.

Copy link
Member

@dpgeorge dpgeorge left a comment

Choose a reason for hiding this comment

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

Thank you for this, a really useful and needed addition!

python-stdlib/datetime/datetime.py Outdated Show resolved Hide resolved
python-stdlib/datetime/datetime.py Outdated Show resolved Hide resolved
python-stdlib/datetime/datetime.py Outdated Show resolved Hide resolved
python-stdlib/datetime/datetime.py Outdated Show resolved Hide resolved
@dpgeorge
Copy link
Member

The tests that you wrote (which look really good), do they pass when run against CPython's datetime module? Ie does the test work under CPython?

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
By matching the order of class methods used in Python's datetime, code
changes are easier to implement.

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
`timedelta` resolution is now 1 nanosecond, instead of 1 second. The
module is now suitable for logging at microsecond scale and below. As a
bonus, `timedelta` can now handle deltas which span ±292 years (instead
of the previous ±68 years). The cost is 4 more bytes per instance.

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
When adding a negative integral number of days, time's reminder was lost
and the result was one day less than expected.

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
MicroPython integer implementation will take care of small and large
deltas.

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
@lorcap
Copy link
Contributor Author

lorcap commented Nov 12, 2021

The tests that you wrote (which look really good), do they pass when run against CPython's datetime module? Ie does the test work under CPython?

No, they don't. You rise a good point, though. I'll see if I can make them run.

@dpgeorge
Copy link
Member

I'll see if I can make them run.

It would be good to try, it might point to some minor inconsistencies between the CPython implementation and the one here.

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
@lorcap
Copy link
Contributor Author

lorcap commented Dec 2, 2021

My first implementation was not compliant with standard datetime in many ways. I decided to take up @dpgeorge's challenge and make a new implementation which now features:

  • full compliance to CPython datetime module
  • full support to classes date and time
  • more methods for timedelta and datetime.

As explained in test_datetime.py, the test script can be run in 3 different modes:

  1. python3 test_datetime.py --stdlib: checks that the tests comply to CPython's standard datetime library.
  2. python3 test_datetime.py: runs the tests against datetime.py, using CPython's standard unittest (which accepts filter options, such as -v TestTimeDelta -k tuple, and provides more verbose output in case of failure).
  3. micropython test_datetime.py: runs the tests against datetime.py using MicroPython's unittest library (which must be available).

The commit above is the result. It still lacks:

  • a full set of test cases for classes date and time (which I'm going to write)
  • folding concept (which comes into play when switching from/to daylight saving time)
  • a few minor class methods which are somehow deprecated in CPython datetime.

My concern is resource usage. The module is getting bigger and bigger. Classes date and time will hardly be used on their own. Their are needed by datetime but only a few methods; all the others are cosmetic (__str__, isoformat, etc.), for comparison (__eq__, etc.) or math (_add__, etc.) methods. I could split the monolithic datetime.py into common.py (which would include classes date_base and time_base), timedelta.py, date.py, time.py, and datetime.py. Is it worth it? How to measure resource usage in Micropython?

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
@lorcap
Copy link
Contributor Author

lorcap commented Dec 17, 2021

I have a question. The fold concept implementation added some complexity to datetime.fromtimestamp() and datetime.timestamp() when dealing with naive objects. The implementation relays on proper management of DST (daylight saving time) for the timezone of interest by mean of time.localtime(). In Unix MP, this is accomplished by properly setting the TZ environment variable. What about supported boards? If none of them provides a time.localtime() supporting DST and TZ, we might remove the code and spare several bytes. Aware objects must be handled by users, anyway. Any suggestion?

Notes:

  • fold is either 0 or 1. It is used to disambiguate wall times during a repeated interval. A repeated interval occurs when clocks are rolled back at the end of daylight saving time.
  • A naive object does not contain enough information to unambiguously locate itself relative to other date/time objects. Naive objects are easy to understand and to work with, at the cost of ignoring some aspects of reality.
  • An aware object is not naive.

Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
Signed-off-by: Lorenzo Cappelletti <lorenzo.cappelletti@gmail.com>
@lorcap
Copy link
Contributor Author

lorcap commented Dec 23, 2021

OK @dpgeorge, I'm done. Last commit 49cd85d removes support to naive datetime objects in datetime.fromtimestamp(), datetime.timestamp() and datetime.astimezone() (see my previous post as to why). A patch file is provided if we change our mind (in case the commit history gets lost). If you want to keep the support, feel free to use commit b4ac923.

The documentation (see PR#7859) is ready, too. I provided some instructions at the end of the file to manually remove date and time classes and other resource greedy methods, because I see little use of them. This saves some precious resources.

@lorcap
Copy link
Contributor Author

lorcap commented Jan 29, 2022

Any chance to get this merged?

@chrisdecker1201
Copy link

I really would love to have this merge request integrated. I'm already using this module and it works fine 😄

@dpgeorge
Copy link
Member

Thanks for all the effort on this. It looks to be in good shape now, and it's great that the tests can run under CPython (with this version and with CPython's standard library version) and MicroPython.

Squashed and merged in fc86070 (and I put the version of this new module at 4.0.0 so it's larger than the existing unix-ffi version of 3.3.3-1).

@dpgeorge dpgeorge closed this Mar 22, 2022
@dpgeorge
Copy link
Member

I saw there were some tuple() methods supported here but not in CPython. What are they used for? Maybe they can be removed?

@lorcap
Copy link
Contributor Author

lorcap commented Mar 22, 2022

I saw there were some tuple() methods supported here but not in CPython. What are they used for? Maybe they can be removed?

CPython provides an interface to extract date/time components via hour(), minute(), day(), month(), etc. Computationally, each component is the remainder of a division chain. If you call hour() and then minute(), this division chain is computed twice. tuple() provides all results at once and represents, IMO, an optimization which MicroPython programmers can take advantage of (if they don't mind compatibility).

@lorcap
Copy link
Contributor Author

lorcap commented Mar 22, 2022

Thanks for all the effort on this. It looks to be in good shape now, and it's great that the tests can run under CPython (with this version and with CPython's standard library version) and MicroPython.

Thanks to you for merging. Please, don't forget the documentation branch (PR #7859)!

@dpgeorge
Copy link
Member

CPython provides an interface to extract date/time components via hour(), minute(), day(), month(), etc. Computationally, each component is the remainder of a division chain. If you call hour() and then minute(), this division chain is computed twice

Does CPython provide any other (efficient) way to get these entries? Maybe .timetuple()?

@lorcap
Copy link
Contributor Author

lorcap commented Mar 22, 2022

Does CPython provide any other (efficient) way to get these entries? Maybe .timetuple()?

CPython is efficient in providing date/time components because it stores those components in the classes. My implementation makes use of big ints and single components need to be computed.

PS: .timetuple() returns an instance of class time.struct_time, pulling in another module.

@dpgeorge
Copy link
Member

.timetuple() returns an instance of class time.struct_time, pulling in another module.

This struct is compatible with a tuple: you can get its length and index it like a tuple. So it would be CPython compatible to implement this method and just make it return a normal tuple, with the same entries in the same order.

@lorcap
Copy link
Contributor Author

lorcap commented Mar 23, 2022

So it would be CPython compatible to implement this method and just make it return a normal tuple, with the same entries in the same order.

.timetuple() is implemented in this porting. It returns a simple 8-tuple because struct_time is not defined in time. .tuple() is an extra feature for MP users which serves also as a base for .tuple(), .day(), .hour(), etc. If you don't like it, we can rename it to ._tuple().

@dpgeorge
Copy link
Member

.timetuple() is implemented in this porting.

Ah, so it is! I didn't notice.

If you don't like it, we can rename it to ._tuple()

I think it would be good to rename .tuple() to ._tuple(). So that it's not part of the official public API. Users can still use ._tuple() if they need efficiency, but at least with the underscore it indicates that this function may change in a future release.

@lorcap
Copy link
Contributor Author

lorcap commented Mar 24, 2022

I think it would be good to rename .tuple() to ._tuple().

OK, even though I don't understand why there couldn't be a documented method targeting microcontrollers. I have a couple of questions:

  1. Where shall I push the change? On top of this branch or with a new pull request?
  2. What about documentation? Can I replace .tuple() with ._tuple() in there, as well? Or shall I get rid of it all together?

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

3 participants