GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Less strict numpy and pyarrow dependencies by Tom-Newton · Pull Request #74 · databricks/databricks-sql-python

Conversation

@Tom-Newton

@Tom-Newton Tom-Newton commented Dec 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Currently this library has quite strict requirements for pyarrow and numpy versions which significantly limits the usefulness. As far as I can tell this library does not require any particular features of numpy or pyarrow added in recent versions.

Resolves: #55

I tested building the wheel locally with poetry build and using it in my project and it worked as expected.

I've had difficulty updating the poetry.lock file though. I've never used poetry before but I think I need to run poetry update. Unfortunately this seems to be taking an extremely long time (> 20 minutes) to the point that I think it must be broken.

Signed-off-by: Thomas Newton [email protected]

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Tom-Newton
Tom-Newton commented Dec 15, 2022
View reviewed changes
Comment thread pyproject.toml Outdated
numpy = [
{version = "1.21.1", python = ">=3.7,<3.8"},
{version = "1.23.4", python = ">=3.8"}
{version = ">=1.16.6", python = ">=3.7,<3.8"},

@Tom-Newton Tom-Newton Dec 15, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It looks like numpy is barely used in this library so I think pretty much any version should work. I chose >=1.16.6 because that is what the latest version of pyarrow requires.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Tom-Newton
Tom-Newton commented Dec 15, 2022
View reviewed changes
Comment thread pyproject.toml
{version = "1.21.1", python = ">=3.7,<3.8"},
{version = "1.23.4", python = ">=3.8"}
{version = ">=1.16.6", python = ">=3.7,<3.11"},
{version = ">=1.23.4", python = ">=3.11"}

@Tom-Newton Tom-Newton Dec 15, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It looks like the special requirement for python 3.11 was added recently in #60 so I assume that is important but I changed it to accept >= to this version.

I also think we should only enforce this requirement on python 3.11

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Tom-Newton
Tom-Newton commented Dec 15, 2022
View reviewed changes
Comment thread pyproject.toml
pandas = "^1.3.0"
pyarrow = [
{version = ">=9.0.0", python = ">=3.7,<3.11"},
{version = ">=6.0.0", python = ">=3.7,<3.11"},

@Tom-Newton Tom-Newton Dec 15, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Probably we could allow even older versions but I think this should make most people happy.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Tom-Newton Tom-Newton changed the title Less strict dependencies Less strict numpy and pyarrow dependencies Dec 15, 2022
@Tom-Newton
Tom-Newton marked this pull request as ready for review December 15, 2022 20:46
@Tom-Newton
Tom-Newton requested review from arikfr, moderakh, susodapop and yunbodeng-db as code owners December 15, 2022 20:46
@Tom-Newton Tom-Newton mentioned this pull request Dec 31, 2022
PyArrow version requirement #55
Closed
Tom-Newton added 2 commits February 13, 2023 21:36
@Tom-Newton
Less strict dependencies
b57d4ea 
Signed-off-by: Thomas Newton <[email protected]>
@Tom-Newton
Fix python version condition
f48ad07 
Signed-off-by: Thomas Newton <[email protected]>
@Tom-Newton

Tom-Newton commented Feb 13, 2023

Copy link
Copy Markdown
Contributor Author

Looks like there is one failure on the CI checks

Download action repository 'actions/checkout@v2' (SHA:dc323e67f16fb5f7663d20ff7941f27f5809e9b6)
Warning: Failed to download action 'https://api.github.com/repos/actions/checkout/tarball/dc[32](https://github.com/databricks/databricks-sql-python/actions/runs/4168062376/jobs/7214961091#step:1:36)3e67f16fb5f7663d20ff7941f27f5809e9b6'. Error: Resource temporarily unavailable (api.github.com:443)
Warning: Back off 22.447 seconds before retry.
Warning: Failed to download action 'https://api.github.com/repos/actions/checkout/tarball/dc323e67f16fb5f7663d20ff7941f27f5809e9b6'. Error: Resource temporarily unavailable (api.github.com:443)
Warning: Back off 24.765 seconds before retry.
Error: Resource temporarily unavailable (api.github.com:443)

I don't think this can be because of my changes. I guess its just a transient thing but I don't think I can re-trigger it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Tom-Newton

Tom-Newton commented Feb 13, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Thanks for triggering the checks @susodapop. It looks like all the unittests passed for all the different python versions. One of the code quality checks is failing though. From the error (Error: Resource temporarily unavailable (api.github.com:443)) I would guess a retry would solve it.

If someone could retry that check and review the PR that would be much appreciated.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Tom-Newton

Tom-Newton commented Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

@susodapop sorry to be impatient but please could you review this? I think this is about to start causing us some significant problems if its not fixed.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@susodapop

susodapop commented Feb 22, 2023

Copy link
Copy Markdown
Contributor

Not impatient at all! We just needed to freeze our dependencies to get v2.4.0 out the door. Else this would have merged already.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@susodapop

susodapop commented Feb 22, 2023

Copy link
Copy Markdown
Contributor

@Tom-Newton can you grant me push on your fork? I need to re-generate the poetry.lock file before I can merge this but I can't push my version to your fork right now.

Also this reminds me that I need to add a segment to our CONTRIBUTING.md file about updating the project dependencies. Unless you regenerate the poetry.lock file then none of the changes are actually picked up during our unit/e2e tests 🤦
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Feb 22, 2023
[Internal] Less strict numpy and pyarrow dependencies #90
Merged
Strict numpy dependency #88
Closed
@Tom-Newton

Tom-Newton commented Feb 22, 2023

Copy link
Copy Markdown
Contributor Author

Superseded by #90
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Tom-Newton Tom-Newton closed this Feb 22, 2023
@arredond arredond mentioned this pull request Oct 7, 2024
Remove upper caps on dependencies #452
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susodapop susodapop Awaiting requested review from susodapop
@arikfr arikfr Awaiting requested review from arikfr
@moderakh moderakh Awaiting requested review from moderakh
@yunbodeng-db yunbodeng-db Awaiting requested review from yunbodeng-db

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PyArrow version requirement

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@Tom-Newton @susodapop
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.