/cc @abalkin @pganssle

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@taleinat @ZackerySpytz cc @pganssle @abalkin

First of all, thanks for the code review.
I 've reflected the review for this PR.

However, other comments have been raised by the core developer on whether this patch is acceptable.
IMHO, It is a good idea to review the patch after the discussion is complete.

Again, thank you for the code reviews. :)

@rhettinger
Is there anything can review with this PR except updating PR naming and updating documentation?

Marking this as closed. See the discussion on BPO where we've decided that collections.namedtuple() wasn't the way to go here. Instead, C functions should use structseq.

Thanks for PR and sorry the effort didn't come to fruition.

@rhettinger This PR uses a structseq, it's what we used for the benchmarks, and I see no reason to close it. Dong-Hee can just push more commits with any changes you might request.

I'm going to re-open this, since @taleinat and I already have review comments on it and it's unclear what you would want @corona10 to do differently here (and even so, comments and pushing new commits would probably be preferable to closing it and opening a new PR).

I changed the title to replace "named tuple" with "structseq". All the code and doc changes checked in here should do something similar - the correct name is "struct sequence" (see, e.g., the existing docs for sys.float_info).

Paul, are you still wanting to go down the path of using collections.namedtuple() instead of structseq? If not, this PR needs to be closed.

Raymond, the actual current code here uses PyStructSequence_XXX calls. Not named tuples anymore. That's why I changed the title 😄.

I had make a quick scan of the latest version and didn't expect to see from collections import namedtuple on line 11 of Lib/datetime.py.

Yes, the Python and C implementations differ in the current state. I just assumed Paul was keen to change the C version to structseqs first, to get his benchmarks done.

Also, I had thought we were going to use "named tuple" (two words) to cover the generic concept including both collections.namedtuple() and structseq. See https://docs.python.org/3/glossary.html#term-named-tuple

I don't think the user ever sees something called a "structseq".

>>> import time
>>> time.localtime()
time.struct_time(tm_year=2019, tm_mon=9, tm_mday=8, tm_hour=16, tm_min=50, tm_sec=17, tm_wday=6, tm_yday=251, tm_isdst=1)
>>> type(_)
<class 'time.struct_time'>

I don't think the API of a structseq is publicly documented anywhere, so it may not make sense to refer to it at all. For example, see what was done for https://docs.python.org/3/library/time.html#time.struct_time

I have made the requested changes; please review again

Thanks for making the requested changes!

@pganssle, @taleinat: please review the changes made to this pull request.

cc @vstinner

@corona10 Hey, sorry I never got back to this, I've been swamped with PEP 615, but I'd love to get this in before the feature freeze.

Do you mind letting me know the current state of things? Do you have a recommendation between this implementation and the other one?

I'm also increasingly amenable to the idea of just having this thing named _IsoCalendar or something (particularly if we can overload the __repr__ to have it return IsoCalendar(...) instead?) if it's too complicated to do anything else.

@pganssle

Hmm, I should remember what I tried before for this issue :)

AFAIK(??) I still prefer the current PR implementation.
Since #17651 is too verbose. (It's just my opinion)
he current implementation is able to compatible with pure python code.
This PR can be the same level of verbose but we can bring compatible issues easily.
(You know namedtuple and structseq is quite different.)

But if you decide specific requirements then we can change the implementation.
And I prefer the start with new PR.
We(or You) can create a new PR to start.

or we have to think that this issue is should be solved?