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

refactor(deps): remove multipledispatch as a dependency #8332

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Feb 13, 2024

This PR removes multipledispatch as a dependency of Ibis. After the-epic-split was merged our largest consumer of multipledispatch was removed, now there is this remaining bit of code to port over to use non-multipledispatch-style checks.

@cpcloud cpcloud added this to the 9.0 milestone Feb 13, 2024
@cpcloud cpcloud added refactor Issues or PRs related to refactoring the codebase dependencies Issues or PRs related to dependencies datatypes Issues relating to ibis's datatypes (under `ibis.expr.datatypes`) labels Feb 13, 2024
@kszucs
Copy link
Member

kszucs commented Feb 13, 2024

The approach looks good to me, going to have a more thorough look.

@@ -46,166 +43,3 @@ def highest_precedence(dtypes: Iterator[dt.DataType]) -> dt.DataType:
return functools.reduce(higher_precedence, collected)
else:
return dt.null


@castable.register(dt.DataType, dt.DataType)
Copy link
Member

Choose a reason for hiding this comment

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

Shall we have function for backward compatibility?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, this isn't a public API.

Copy link
Member

@kszucs kszucs Feb 13, 2024

Choose a reason for hiding this comment

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

How about removing dt.cast() as well then in favor of the cast() method?

Copy link
Member Author

Choose a reason for hiding this comment

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

That one I would rather do in a follow up

ibis/expr/datatypes/core.py Outdated Show resolved Hide resolved
@cpcloud cpcloud force-pushed the remove-multipledispatch branch 2 times, most recently from 99b42f3 to bb1a2c5 Compare February 13, 2024 12:52
@kszucs kszucs merged commit d587166 into ibis-project:main Feb 13, 2024
75 checks passed
ncclementi pushed a commit to ncclementi/ibis that referenced this pull request Feb 21, 2024
…ct#8332)

This PR removes `multipledispatch` as a dependency of Ibis. After
`the-epic-split` was merged our largest consumer of `multipledispatch`
was removed, now there is this remaining bit of code to port over to use
non-multipledispatch-style checks.
@cpcloud cpcloud deleted the remove-multipledispatch branch July 16, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datatypes Issues relating to ibis's datatypes (under `ibis.expr.datatypes`) dependencies Issues or PRs related to dependencies refactor Issues or PRs related to refactoring the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants