Skip to content

DEPS: Revert SQLAlchemy minimum version back to 1.4.36 #60977

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

Closed
wants to merge 27 commits into from

Conversation

snitish
Copy link
Member

@snitish snitish commented Feb 21, 2025

@snitish snitish requested a review from mroeschke as a code owner February 21, 2025 07:41
@snitish snitish marked this pull request as draft February 21, 2025 07:41
@snitish snitish changed the title BUG: Revert SQLAlchemy minimum version back to 1.4.36 DEPS: Revert SQLAlchemy minimum version back to 1.4.36 Feb 21, 2025
@snitish
Copy link
Member Author

snitish commented Feb 21, 2025

@mroeschke given there've been so many requests for this, thought I'd give this a try. The minimum version for SQLAlchemy was bumped from 1.4.36 to 2.0.0 about a year ago (#55524). I'm resetting it back to 1.4.36.
My local tests ran fine with this version but is there a way to configure CI tests to run on both SQLAlchemy versions?

@swarajban
Copy link

I don't know if just changing the versions will fix the underlying issues. When we use an old version of SA, functions like pandasSQL_builder don't parse the connection object properly and return SQLite (which causes other downstream issues, eg params to not work correctly)

@snitish
Copy link
Member Author

snitish commented Feb 21, 2025

I don't know if just changing the versions will fix the underlying issues. When we use an old version of SA, functions like pandasSQL_builder don't parse the connection object properly and return SQLite (which causes other downstream issues, eg params to not work correctly)

The reason it was incorrectly returning an SQLite object is because of this line:

sqlalchemy = import_optional_dependency("sqlalchemy", errors="ignore")

If SQLAlchemy doesn't satisfy the minimum version, the import fails and it leads to creating an SQLite object instead. If the minimum version is updated, the code works fine. Tested it on my branch after updating SQLAlchemy to 1.4.

>>> import pandas as pd
>>> from sqlalchemy import create_engine
f = pd.DataFrame({'a': [0, 1, 2, 3]})
df.to_sql('test', con)>>> con = create_engine('sqlite:///:memory:')
>>> df = pd.DataFrame({'a': [0, 1, 2, 3]})
>>> df.to_sql('test', con)
4

@swarajban
Copy link

I don't know if just changing the versions will fix the underlying issues. When we use an old version of SA, functions like pandasSQL_builder don't parse the connection object properly and return SQLite (which causes other downstream issues, eg params to not work correctly)

The reason it was incorrectly returning an SQLite object is because of this line:

sqlalchemy = import_optional_dependency("sqlalchemy", errors="ignore")

If SQLAlchemy doesn't satisfy the minimum version, the import fails and it leads to creating an SQLite object instead. If the minimum version is updated, the code works fine. Tested it on my branch after updating SQLAlchemy to 1.4.

>>> import pandas as pd
>>> from sqlalchemy import create_engine
f = pd.DataFrame({'a': [0, 1, 2, 3]})
df.to_sql('test', con)>>> con = create_engine('sqlite:///:memory:')
>>> df = pd.DataFrame({'a': [0, 1, 2, 3]})
>>> df.to_sql('test', con)
4

Ah wow, well gl! We would really appreciate this change

@mroeschke
Copy link
Member

is there a way to configure CI tests to run on both SQLAlchemy versions?

There is a Minimum Versions build that will test with the pinned, minimum supported version of the dependencies (1.4.x), and the rest of the builds test with dependencies pinned as >=version so those builds should test with 2.0.x.

@snitish
Copy link
Member Author

snitish commented Feb 22, 2025

Thanks @mroeschke . Confirmed that the minimum version tests have passed. The failing Ubuntu tests are apparently due to an unrelated issue in libsqlite-3.49.1. (See AUTOMATIC1111/stable-diffusion-webui#16856)
I'm still not sure why these tests are passing for other PRs - they seem to be using the older 3.48.1 version - but my guess is that updating the minimum versions file may have triggered the build to re-fetch the latest versions of dependencies.

@snitish snitish marked this pull request as ready for review February 23, 2025 23:05
@mroeschke
Copy link
Member

Could you add a whatsnew note in v2.3.0? I think it's reasonable to backport this since many users were reporting this issue

@mroeschke
Copy link
Member

And after this PR, do you mind submitting one to our conda-feedstock to updating sqlalchemy version there? https://github.com/conda-forge/pandas-feedstock/blob/main/recipe/meta.yaml

@mroeschke mroeschke added IO SQL to_sql, read_sql, read_sql_query Dependencies Required and optional dependencies labels Feb 26, 2025
@mroeschke mroeschke added this to the 2.3 milestone Feb 26, 2025
@snitish
Copy link
Member Author

snitish commented Feb 26, 2025

@mroeschke should the whatsnew note go under the 'Other Enhancements' section? Or should I create a new section for minimum version updates?

@mroeschke
Copy link
Member

should the whatsnew note go under the 'Other Enhancements' section?

Yup, I think this section is appropriate

@jabbera
Copy link

jabbera commented Mar 11, 2025

Is this going to be merged?!?! I cannot tell you how excited this makes me. The amount of pain this issue has caused me knows no bound.

Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Apr 11, 2025
@miraculixx
Copy link

miraculixx commented Apr 16, 2025

Much appreciate this work. How can we help to get this merged and included in a release? 🙏🏼

@simonjayhawkins
Copy link
Member

pre-commit.ci autofix

Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label May 19, 2025
@mroeschke mroeschke modified the milestones: 2.3, 3.0 Jun 2, 2025
@datapythonista
Copy link
Member

Sorry @snitish this wasn't merged. Can you move the release note back to 3.0 and resolve the conflicts please, so we can still get this for 3.0. Thanks!

snitish and others added 25 commits June 8, 2025 19:21
…ow dtype (pandas-dev#61229)

* BUG: Fix pandas-dev#61222: Keep index name when resampling with pyarrow dtype

* Update doc/source/whatsnew/v3.0.0.rst

---------

Co-authored-by: Matthew Roeschke <[email protected]>
Added docstrings to min, max, and reso

Co-authored-by: John Hendricks <[email protected]>
* Bump pre-commit version, bump clang-format and meson

* Fix type checking abbreviation

* Bump to 0.11.4

* Put minimum version at 4

* Change misc to arg-type
…-dev#61162)

* updated indexing.py to allow iloc.__getitem__

* Updated test_iloc_mask test

* bugfix test_iloc_mask test

* bugfix test_iloc_mask

* whatsnew

* added test to test_iloc_mask

* formatting

* precommit

* added tests for series bool mask

* precommit

* reformatted tests
…#61244)

* Refactor time series plotting logic for improved clarity

Extract and streamline time series preparation steps into `prepare_ts_data`, replacing redundant logic across methods. Simplifies axis frequency handling and improves code readability while maintaining functionality.

* Add test to validate xtick alignment for scatter and line plots

This test ensures that the x-axis ticks are consistent between scatter and line plots when sharing the same axis. It addresses a potential issue related to GH#61005, verifying proper rendering of datetime x-axis labels.

* Fix bug in Series.plot misalignment for line and scatter plots

This resolves an issue where line and scatter plots were not aligned when using Series.plot. The fix ensures proper alignment and improves plot consistency. Refer to issue pandas-dev#61005 for further details.

* Update scatter plot test to support datetime.time data

Datetime.time is now supported in scatter plots due to added converter implementation in ScatterPlot. Removed the test expecting a TypeError and updated it to validate the new functionality.

* Refactor handling of x_data in matplotlib plotting.

Simplify and streamline the code by directly assigning x_data from the data variable and replacing the intermediate Series object with a clearer `s` variable. This improves readability and maintains the existing functionality.

* Move test_scatter_line_xticks from Series to DataFrame tests

Relocated the `test_scatter_line_xticks` test from `test_series.py` to `test_frame.py` for better alignment with DataFrame-specific functionality. This refactor ensures the test resides in the appropriate context based on its usage and focus.

* Refactor `prepare_ts_data` to improve type annotations.

Added precise type annotations to the function signature for better clarity and type checking. Replaced `data` with `series` and `kwds` with `kwargs` to enhance readability and consistency.

* Refactor test_scatter_line_xticks to simplify DataFrame creation

The DataFrame creation in the test has been streamlined for clarity and conciseness by replacing the loop with a list comprehension. This improves code readability and maintains the same functionality.

* Refactor Series import to optimize scope and maintain consistency

Moved the `Series` import inside relevant function scopes to minimize unnecessary top-level imports and align with existing import patterns. This helps improve code readability and ensures imports are only loaded where needed.

* `Reorder import statement in _make_plot method`

Moved the import of `Series` within the `_make_plot` method to comply with styling or runtime considerations. This ensures consistency and avoids potential import-related issues.
…ndas-dev#61249)

* BLD: Try pinning ninja<1.11.1.4 for windows free threading build

* quote

* change to triple and double quotes

* Add in script instead

* Add in script instead

* Try numpy 2.2.3

* Double quotes

* Try 2.2.2

* Install older Cython

* Try commit from March 17

* try march 19 commit 0b866bf7d43ced968dba4e9726316f963aae8f3c

* Try march 18 commit b4917f731da50062f8ba53737ade7b82b4c8fcf2

* One commit after c compiler warnings PR

* Use March 20 commit 93a7d09d47d8aae0dfcea41d06f4b140a1161499

* Use cb156c48d94b7e13363ab791b16bdeeb3392f21e before vector call

* One more divmod commit

* USe divmod commit

* Use commit before divmod, undo ninja and numpy changes
* Changed term non-null to NA

* Update pandas/io/formats/info.py

Co-authored-by: Matthew Roeschke <[email protected]>

---------

Co-authored-by: Matthew Roeschke <[email protected]>
CI Maybe fix Windows free-threaded

Co-authored-by: Matthew Roeschke <[email protected]>
* TYP: Add ignores for numpy 2.2 updates

* fix tests and plotting

* ignore pyright error
…rectly for full error details (pandas-dev#61084)

* Add hint to display full message for missing dependencies in pandas/init.py

* ENH: Improve import error handling to preserve original traceback

* TST: refactor testing for hard dependency package

* Update pandas/__init__.py

Co-authored-by: Matthew Roeschke <[email protected]>

* Refactor prevent statement too long

* ENH: change unittest to verify ImportError is raised when required dependencies are missing

* TST: Use pytest.raises match parameter in test_missing_required_dependency

---------

Co-authored-by: Matthew Roeschke <[email protected]>
* PERF: stack on non-MultiIndex columns

* WIP

* Use reshape instead of ravel

* arrays -> blocks

* Update test

* whatsnew
…as-dev#61208)  (pandas-dev#61216)

* Fix pandas-dev#61208: OverflowError when fillna on DataFrame with a pd.Timestamp
- Now correctly raises OutOfBoundsDatetime
- Added test_fillna_out_of_bounds_datetime()

* Comply with pre-commit and added an entry in v3.0.0.rst

* Removed flag 'inplace=True' from test and fixed the bug for this case.
…andas-dev#61193)

* BUG: Fix bug with DataFrame.pivot and .set_index not compatible with pyarrow dictionary categoricals

Relates to pandas-dev#53051

Code for fix taken and adapted from pandas-dev#59099

* TST: Add tests for faulty behavior relating to pyarrow categoricals

* CLN: Fix issues reported by pre-commit hooks

* TST: Fix failing tests for minimum version by ignoring obsolete deprecation warning

* DOC: Add entry for bugfix to whatsnew v3.0.0

* CLN: Refactor code and clean up according to PR feedback

* CLN: Refactor code and clean up according to PR feedback

* CLN: Refactor tests to adress PR feedback

* CLN: Refactor tests to adress PR feedback
* updated doc and references

* precommit

* shortened summary

* updated according to reviewer suggestions and removed test.py

* Fixed docstring error
…ev#61116)

* Save original index and remap after function completes.

* precommit passes

* use stable sorting 'mergesort' in tests

* Change sorts to `stable` instead of mergesort

* modify 'keep' to use a Literal instead of string

* address comments

* update doc to include stable sort change
No need to restate the library name
@jmaupetit
Copy link

@snitish why closing this? Should we understand that SQLAlchemy compatibility will never be restored?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Required and optional dependencies IO SQL to_sql, read_sql, read_sql_query Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Pandas 2.2 breaks SQLAlchemy 1.4 compatibility