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

Feature/accept datetime on retrieve #574

Conversation

AlcibiadesCleinias
Copy link
Contributor

resolve #53

But: I did not update docs. Not sure how to do it right

PS.
It is better to review and merge after #572 coz the pr consists of some features of 572 pr

Copy link
Owner

@Hugovdberg Hugovdberg left a comment

Choose a reason for hiding this comment

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

I would like you to make a few changes to the new time module to keep Python 2.7 support for now. I like that you put this in a separate module, I think that the PISeries.timestamp_to_index function could be moved in there as well.

By the way, I already made a new branch remove_python2_support to which should be Python 3 only, and include type hints everywhere, feel free to post contributions to that branch if you want to help strip out the Python 2 compatibility. Once PIconnect 0.9 is released I want to merge that branch into develop.

@AlcibiadesCleinias
Copy link
Contributor Author

I think that the PISeries.timestamp_to_index function could be moved in there as well.

Think the same: done

By the way, I already made a new branch remove_python2_support to which should be Python 3 only, and include type hints everywhere, feel free to post contributions to that branch if you want to help strip out the Python 2 compatibility

Seems like a good idea. I will checkout

@AlcibiadesCleinias
Copy link
Contributor Author

oh, ought to mention: no docs about new module generated...

@Hugovdberg
Copy link
Owner

I think not documenting the new module is fine for now, as it is meant to be used internally only. I'm working on improving the documentation structure so it becomes more clear where to add the new documentation.

@Hugovdberg Hugovdberg merged commit 5167777 into Hugovdberg:develop Jul 29, 2021
@Hugovdberg Hugovdberg mentioned this pull request Aug 10, 2021
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.

Feature request: accept datetime objects for date range selection
2 participants