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

Decommission time.py from the server Python API #4214

Closed
jmao-denver opened this issue Jul 21, 2023 · 0 comments · Fixed by #4388
Closed

Decommission time.py from the server Python API #4214

jmao-denver opened this issue Jul 21, 2023 · 0 comments · Fixed by #4388
Assignees
Labels
feature request New feature or request
Milestone

Comments

@jmao-denver
Copy link
Contributor

jmao-denver commented Jul 21, 2023

Reasons:

  1. After switching from home-grown DH DateTime type to Java's own Instant and siblings to store date/time data in DH, the utility provided by time.py could be achieved by 3rd party/DH built conversion support between Java Instant and Python DateTime and Python's own DateTIme manipulation libraries.
  2. A number of the wrapper functions in time.py, including lower_bin(), upper_bin(), now(), when used in queries, make the execution 20x+ slower because of the double-crossing between JVM/Python. The effort to automatically substitute them with their Java camelCase counterparts has proven to be messy and unsatisfactory.
  3. Since these utility functions should not be considered as typical Python UDFs, but rather part of the built-ins of DH query language, it is arguably better/more acceptable to clearly document them and educate the user on how to use them than for purist reasons to wrap them in Python but making it confusing for the users and trying to solve the performance problem that could be avoided otherwise.

This would also render following #2777, #2306, #2303 non-issues.

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

Successfully merging a pull request may close this issue.

2 participants