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

chore: Update python files #1141

Closed
wants to merge 2 commits into from

Conversation

milenkovicm
Copy link
Contributor

update pip dependencies, pyproject and cargo, also added fix few small issues.

Which issue does this PR close?

Closes #.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@milenkovicm milenkovicm marked this pull request as draft November 27, 2024 18:22
- pip
- cargo

and fix few small issues.
.standalone()

#ctx_remote: SessionContext = ballista.remote("remote_ip", 50050)
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know if we wanted to show an example of connecting to a remote instance, but it is covered in the other docs anyway, so good call removing.

build-backend = "maturin"

[project]
name = "ballista"
description = "Python client for Apache Arrow Ballista Distributed SQL Query Engine"
readme = "README.md"
license = {file = "LICENSE.txt"}
requires-python = ">=3.6"
requires-python = ">=3.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

This would conclict with #1136 so I will change it really quick and repush my branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

updated #1136 to require python 3.7 or greater

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Datafusion python has python > 3.7

@@ -66,7 +63,11 @@ impl PyBallistaBuilder {
/// Construct the standalone instance from the SessionContext
pub fn standalone(&self, py: Python) -> PyResult<PySessionContext> {
// Build the config
let config: SessionConfig = SessionConfig::from_string_hash_map(&self.conf)?;
let mut config: SessionConfig = SessionConfig::new_with_ballista();
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you update something in new_with_ballista? When I initially tried this I was getting an error. If it works thatn that is great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed nothing

Copy link
Contributor

Choose a reason for hiding this comment

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

This was initially used when awaiting the old BallistaContext. I think we could remove if needed. I left it in because I was unsure if we would need it again, but at this point I think we could just delete.

Copy link
Contributor

@tbar4 tbar4 left a comment

Choose a reason for hiding this comment

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

Looks good. Only callout is potentially removing to_pyerr

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

Successfully merging this pull request may close these issues.

2 participants