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

Implement DayTimeInterval and YearMonthTimeInterval types #512

Merged

Conversation

guilhermeleobas
Copy link
Contributor

No description provided.

@guilhermeleobas guilhermeleobas added enhancement New feature or request heavydb Related to heavydb server labels Oct 14, 2022
@guilhermeleobas guilhermeleobas self-assigned this Oct 14, 2022
@guilhermeleobas guilhermeleobas force-pushed the guilhermeleobas/day_year_time_interval branch from 4d65deb to 4a6f92d Compare October 14, 2022 03:26
@guilhermeleobas guilhermeleobas marked this pull request as ready for review October 14, 2022 15:20
Copy link
Contributor

@pearu pearu left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @guilhermeleobas !

Comment on lines +734 to +735
def __hash__(self) -> int:
return super().__hash__()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to define __hash__ here while the base class (tuple) already provides it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without it, the following fails:

(Pdb++) hash(Type('float64'))
*** TypeError: unhashable type: 'Type'

@pearu pearu merged commit 3c7ed45 into heavyai:main Nov 7, 2022
@guilhermeleobas guilhermeleobas deleted the guilhermeleobas/day_year_time_interval branch January 24, 2023 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request heavydb Related to heavydb server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants