Skip to content

Add timespec argument to to_iso8601_string() function #353

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Itay4
Copy link

@Itay4 Itay4 commented Mar 9, 2019

Fixes #347

@Itay4
Copy link
Author

Itay4 commented Mar 9, 2019

currently python 2.7 build fails, as datetime for it does not support timespec argument
how would you like me to handle that?
should i check the version and split code accordingly ?

@joaonc
Copy link

joaonc commented Mar 15, 2019

I would love to see this PR merged, however doing it would mean that the behavior would be different for Python 3.6+ and lower versions (note: not just python 2.7, as this argument was introduced in 3.6).

On the good side, the 3.5- users wouldn't see any change, they just wouldn't be able to use the new feature.

The options are:

  1. Leave as-is and the new parameter would only be available in 3.6+ (easiest).
    This would probably mean check for version and split code.
  2. Implement the whole timespec functionality in pendulum and make it available for everyone.
    This would open a whole new can of worms, probably (haven't checked the implementation in the datetime package.

My suggestion is going with option 1 and make a note of it in the documentation.
In 3.5-, take the argument but do nothing with it.

@Itay4
Copy link
Author

Itay4 commented Mar 16, 2019

Can someone assist and explain why Travis build fails ?

@joaonc
Copy link

joaonc commented Jul 8, 2019

@Itay4 looks like it was the linting step that failed, meaning the warning that black is issuing here is causing the build to fail. Something with code style, is my guess.

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.

Add timespec argument to to_iso8601_string()
2 participants