Skip to content

bpo-40643: Update documentations of strftime function for date, time and datetime. #22477

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

Closed
wants to merge 1 commit into from
Closed

bpo-40643: Update documentations of strftime function for date, time and datetime. #22477

wants to merge 1 commit into from

Conversation

eiffel-fl
Copy link

@eiffel-fl eiffel-fl commented Oct 1, 2020

Hi.

First, I do hope you are all fine and the same for your relatives.

I did not open an issue on cpython bug tracking but this commit normally helps pandas-dev/pandas#36745 and also python developer.
I just simply update the documentation for strftime function for date, time and datetime.
For datetime, since it seems to inherits this function from date I had to add a new PyMethodDef entry.

If there is any problem with the code do not hesitate to say it, so I will improve it.

Best regards.

https://bugs.python.org/issue40643

The documentation is now clearer than before.

This commit should help pandas-dev/pandas#36745.
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@eiffel-fl

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@taleinat
Copy link
Contributor

taleinat commented Oct 2, 2020

Hi @eiffel-fl, thanks for this PR!

Do note that there is already an open issue about these, bpo-40643. There is also an existing PR on this subject, GH-20677.

I suggest combining efforts on getting the previous PR into working shape, especially considering that it currently only alters the doc-strings for the pure-Python implementation, and this only alters the doc-strings for the C implementation!

@taleinat taleinat changed the title Update documentations of strftime function for date, time and datetime. bpo-40643: Update documentations of strftime function for date, time and datetime. Oct 2, 2020
@eiffel-fl
Copy link
Author

Hi @taleinat!

I did not see that there was already an issue and an associated PR, I will be more attentive next time.

I am not really sure but maybe both PR are needed.
If my understanding is correct, in pandas (see pandas-dev/pandas#36745), they inherit datetime.datetime so they also inherit the C doc-strings?
If I am wrong you can close this PR as #20677 solves also the pandas problem.

@taleinat
Copy link
Contributor

taleinat commented Oct 2, 2020

@eiffel-fl,

I did not see that there was already an issue and an associated PR, I will be more attentive next time.

No need to apologize! In this case I happened to have reviewed that PR, so I immediately noticed that this one is related. That being said, searching for existing PRs and issues on a subject is certainly something you should always do (not only for this project). So it seems that you've learned / been reminded of something important :)

I am not really sure but maybe both PR are needed.

Ideally we should make one PR which identically improves the doc-strings for both the C and Python implementations of these classes. Given the existing reviews and discussion on the previously existing PR (GH-20677), and the fact that it was created first, I think we should work on that one.

You can certainly pitch in to working on that PR, e.g. by creating a PR to be merged into it. You won't be the author of the squashed commit, but you'll be tagged with a special tag that will mark you as an additional author. (If this was a feature or bug-fix then you would also be mentioned in the NEWS item.)

@eiffel-fl
Copy link
Author

@taleinat

Ideally we should make one PR which identically improves the doc-strings for both the C and Python implementations of these classes. Given the existing reviews and discussion on the previously existing PR (GH-20677), and the fact that it was created first, I think we should work on that one.

I agree with this point.

You can certainly pitch in to working on that PR, e.g. by creating a PR to be merged into it. You won't be the author of the squashed commit, but you'll be tagged with a special tag that will mark you as an additional author. (If this was a feature or bug-fix then you would also be mentioned in the NEWS item.)

Not being the author of the commit is not a problem for me, the most important thing is to increase code quality.
So, I would like to do this, the only problem is that I do not really see how to do it...
Should I do PR to @SimiCode fork then he does a PR with his and my commits?

@taleinat
Copy link
Contributor

taleinat commented Oct 2, 2020

Not being the author of the commit is not a problem for me, the most important thing is to increase code quality.

😄 👍

Should I do PR to @SimiCode fork then he does a PR with his and my commits?

Indeed, I think that would be the best way.

(For small changes, making suggestions using GitHub's interface is much simpler.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants