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

Allow construction of struct_time from another struct_time #4931

Merged
merged 5 commits into from
Jul 1, 2021

Conversation

jepler
Copy link
Member

@jepler jepler commented Jun 27, 2021

Closes: #4917 -- alternative to #4930.

I suspect this is the leaner alternative. The enhancement to mp_obj_get_array should be considered for upstreaming.

testing performed (nrf52840):

>>> t = time.struct_time(1,2,3,4,5,6,7,8,9)
>>> time.struct_time(t)
struct_time(tm_year=1, tm_mon=2, tm_mday=3, tm_hour=4, tm_min=5, tm_sec=6, tm_wday=7, tm_yday=8, tm_isdst=9)
>>> time.struct_time(tuple(t))
struct_time(tm_year=1, tm_mon=2, tm_mday=3, tm_hour=4, tm_min=5, tm_sec=6, tm_wday=7, tm_yday=8, tm_isdst=9)
>>> time.struct_time(list(t))
struct_time(tm_year=1, tm_mon=2, tm_mday=3, tm_hour=4, tm_min=5, tm_sec=6, tm_wday=7, tm_yday=8, tm_isdst=9)

@jepler jepler requested a review from microdev1 June 27, 2021 01:10
Copy link
Collaborator

@microdev1 microdev1 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a couple things that need to be addressed.

shared-bindings/time/__init__.c Show resolved Hide resolved
shared-bindings/time/__init__.c Show resolved Hide resolved
INCOMPATIBLE CHANGE: struct_time(1,2,3,4,5,6,7,8,9) is now _rejected_
just as on standad Python.

This incorrect constructor was added by me in adafruit#2327; I assumed
without even checking that the `struct_time` constructor was also
compatible with the `namedtuple` constructor, but it is not and has
always been rejected by standard Python (checked 2.7 and 3.9)

This commit restores the specific error message that we used for this
purpose, which was removed in the previous commit either out of laziness
or out of trying to reduce unneeded error strings. In this case, the
alternate string is too misleading (it refers to arguments, not to
sequence elements) so let's put the better message back.
@jepler jepler requested a review from microdev1 June 29, 2021 00:57
Copy link
Collaborator

@microdev1 microdev1 left a comment

Choose a reason for hiding this comment

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

@jepler Thanks for making the requested changes. I pushed a commit to fix a minor issue with the docs.

@dhalbert dhalbert merged commit 7f39779 into adafruit:main Jul 1, 2021
@jepler jepler deleted the struct-time-construct branch November 3, 2021 21:10
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.

time.struct_time doesn't take a list in the constructor
3 participants