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

ENH: Not silently ignore violation of minimal required version of SQLAlchemy #57178

Open
1 of 3 tasks
bebach opened this issue Jan 31, 2024 · 5 comments
Open
1 of 3 tasks
Labels
Dependencies Required and optional dependencies Enhancement IO SQL to_sql, read_sql, read_sql_query

Comments

@bebach
Copy link

bebach commented Jan 31, 2024

Feature Type

  • Adding new functionality to pandas

  • Changing existing functionality in pandas

  • Removing existing functionality in pandas

Problem Description

Observation

After upgrading to Pandas 2.2.0 calling read_sql_query with a SQLAlchemy query on an SQLAlchemy connection yielded "Query must be a string unless using sqlalchemy." without any indication why or what may have broken with the upgrade.

Analysis

Tracking the problem down it was found that pandasSQL_builder (code) calls sqlalchemy = import_optional_dependency("sqlalchemy", errors="ignore") to resolve the optional SQLAlchemy package (which was installed in my case)

Due to errors="ignore" the violation of the minimal required version for SQLAlchemy does not lead to a warn or raise (code) but just silently returns with None.

This in turn lets pandasSQL_builder silently default to SQLiteDatabase(con).

Feature Description

Proposed improvement

Do not ignore the minimal version violation in import_optional_dependency in this context but make it obvious. For example by introducing an additional "errors"-mode like import_optional_dependency("sqlalchemy", errors="raise-on-version-violation").

Alternative Solutions

./.

Additional Context

No response

@bebach bebach added Enhancement Needs Triage Issue that has not been reviewed by a pandas team member labels Jan 31, 2024
@bebach bebach changed the title ENH: ENH: Not silently ignore violation of minimal required version of SQLAlchemy Jan 31, 2024
@rhshadrach
Copy link
Member

Thanks for the report! It looks like this was changed in #45679 to allow for other dependencies. Perhaps the logic could be changed there so that when SQLAlchemy is used, we do check that the version is satisfied.

Further investigations and PRs to fix are welcome!

@rhshadrach rhshadrach added IO SQL to_sql, read_sql, read_sql_query Dependencies Required and optional dependencies and removed Needs Triage Issue that has not been reviewed by a pandas team member labels Feb 1, 2024
@luke396
Copy link
Contributor

luke396 commented Feb 2, 2024

I have attempted to recreate the issue in a fresh environment with pandas==2.2.0; sqlalchemy==2.0.25, and the output seems to be correct. I may require assistance in reproducing the issue.

from sqlalchemy import create_engine
import pandas as pd

engine = create_engine("sqlite:///:memory:")

df = pd.DataFrame({
    "id": [1, 2, 3],
    "name": ["Alice", "Bob", "Charlie"]
})
df.to_sql("Person", engine, index=False)

sql_query = "SELECT * FROM Person"
result = pd.read_sql_query(sql_query, engine)
print(result)
   id     name
0   1    Alice
1   2      Bob
2   3  Charlie

@bebach
Copy link
Author

bebach commented Feb 2, 2024

The issue is not that it works with SQLAlchemy 2 but that it gives no clue why it does not work with SQLAlchemy < 2.x

To replicate install SQLAlchemy==1.4 and rerun the code-snippet from above. This results in..

$ python pandas_test.py
D:\temp\pandas-issue-57178\pandas_test.py:10: UserWarning: pandas only supports SQLAlchemy connectable (engine/connection) or database string URI or sqlite3 DBAPI2 connection. Other DBAPI2 objects are not tested. Please consider using SQLAlchemy.
  df.to_sql("Person", engine, index=False)
Traceback (most recent call last):
  File "D:\temp\pandas-issue-57178\pandas_test.py", line 10, in <module>
    df.to_sql("Person", engine, index=False)
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\util\_decorators.py", line 333, in wrapper
    return func(*args, **kwargs)
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\core\generic.py", line 3081, in to_sql
    return sql.to_sql(
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\io\sql.py", line 842, in to_sql
    return pandas_sql.to_sql(
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\io\sql.py", line 2851, in to_sql
    table.create()
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\io\sql.py", line 984, in create
    if self.exists():
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\io\sql.py", line 970, in exists
    return self.pd_sql.has_table(self.name, self.schema)
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\io\sql.py", line 2866, in has_table
    return len(self.execute(query, [name]).fetchall()) > 0
  File "D:\temp\pandas-issue-57178\venv\lib\site-packages\pandas\io\sql.py", line 2673, in execute
    cur = self.con.cursor()
AttributeError: 'Engine' object has no attribute 'cursor'

which is surprising given that we just created the engine with SQLAlchemy. And it gives the user no clue that this is due to violation of version requirement.

It would be obvious what is the issue if the version check in import_optional_dependency raises the

            msg = (
                f"Pandas requires version '{minimum_version}' or newer of '{parent}' "
                f"(version '{version}' currently installed)."
            )

... instead of just silently returning None (which only later leads to the miss-leading error from above.)

@luke396
Copy link
Contributor

luke396 commented Feb 3, 2024

Seems related #57049, whether to support Sqlalchemy 1.4 is under discussion.

@turbotimon
Copy link

I think this is related. It should state that sqlalchemy <2.0 is not supported instead of "not installed".

import sqlalchemy
print("sqlalchemy Version:", sqlalchemy.__version__)
pd.read_sql("select * from osm_point limit 1", connection_string)
sqlalchemy Version: 1.4.52
...
ImportError: Using URI string without sqlalchemy installed.

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

No branches or pull requests

4 participants