sadpandajoe commented on code in PR #45823:
URL: https://github.com/apache/superset/pull/45823#discussion_r4422909325
##########
superset/utils/core.py:
##########
@@ -2131,13 +2131,24 @@ def _process_datetime_column(
# Parse with or without format (suppress warning if no format)
if format_to_use:
- converted = pd.to_datetime(
- df[col.col_label],
- utc=False,
- format=format_to_use,
- errors="coerce",
- exact=False,
- )
+ try:
+ converted = pd.to_datetime(
+ df[col.col_label],
+ utc=False,
+ format=format_to_use,
+ errors="coerce",
+ exact=False,
+ )
+ except ValueError as ex:
+ if "Mixed timezones" not in str(ex):
+ raise
+ converted = pd.to_datetime(
+ df[col.col_label],
+ utc=True,
Review Comment:
On the `Mixed timezones` fallback the column is converted to UTC, and
`_coerce_temporal_join_series` later strips the tz, so the join compares UTC
wall clocks. A comparison series with a single offset keeps its local wall
clock through the same path. For a `%z` column spanning a DST change (noon at
`-05:00` and noon at `-04:00`), the source becomes 17:00/16:00 while the
single-offset side stays 12:00, so rows that matched by local time no longer
join. Is UTC the intended basis for these joins, or should each value keep its
own local wall clock?
##########
tests/unit_tests/commands/databases/columnar_reader_test.py:
##########
@@ -141,7 +141,7 @@ def test_excel_reader_wrong_columns_to_read():
with pytest.raises(DatabaseUploadFailed) as ex:
reader.file_to_dataframe(create_columnar_file(COLUMNAR_DATA))
assert (
- str(ex.value)
+ str(ex.value).replace(": large_string", ": string")
Review Comment:
The actual message is normalised with `.replace(": large_string", ":
string")`, but the expected literal below still contains `: large_string` for
`Name`, `City` and `Birth`. After the replace the left side can never contain
`large_string`, so this equality fails whichever Arrow type name the reader
reports, including when it correctly raises `DatabaseUploadFailed` for `xpto`.
Should the same normalisation be applied to the expected string (or the
assertion pinned to the stable `No match for FieldRef.Name(xpto)` prefix)?
##########
superset/commands/dataset/importers/v1/utils.py:
##########
@@ -986,32 +986,31 @@ def _convert_temporal_columns(df: pd.DataFrame, dtype:
dict[str, Any]) -> None:
for column_name, sqla_type in dtype.items():
if isinstance(sqla_type, (Date, DateTime)):
try:
- df[column_name] = pd.to_datetime(df[column_name])
- except OutOfBoundsDatetime:
- # Row-level fallback: coerce only OOB values; re-raise for
malformed
- # strings. Whole-column errors="coerce" would silently swallow
- # malformed values that happen to share a column with an OOB
date.
- original = df[column_name].copy()
- result = []
- for val in original:
- if pd.isna(val):
- result.append(pd.NaT)
- continue
- try:
- result.append(pd.to_datetime(val))
- except OutOfBoundsDatetime:
- result.append(pd.NaT)
- # Other exceptions (e.g. malformed strings) propagate
- converted = pd.Series(result, index=original.index)
- n_coerced = int(converted.isna().sum() - original.isna().sum())
- if n_coerced > 0:
- logger.warning(
- "Coerced %d out-of-bounds datetime value(s) "
- "in column '%s' to NaT",
- n_coerced,
- column_name,
- )
- df[column_name] = converted
+ converted = pd.to_datetime(df[column_name])
+ if pd.api.types.is_datetime64_any_dtype(converted):
+ df[column_name] = converted.dt.as_unit("ns")
Review Comment:
Forcing `as_unit("ns")` here (and in the per-value path below) turns a valid
far-future date such as `3118-01-01` into `NaT`: the nanosecond cast overflows,
the `ValueError` is swallowed, the fallback overflows again, and `load_data()`
then writes NULL for that cell. The neighbouring
`test_far_future_date_strings_convert_successfully` asserts the timestamp
survives with no warning, so that test should fail against this code. Could the
conversion keep the coarser resolution pandas already returned instead of
casting to ns?
##########
.github/workflows/python-metadata-compatibility.yml:
##########
@@ -0,0 +1,124 @@
+name: Python-Metadata-Compatibility
+
+permissions:
+ contents: read
+
+on:
+ pull_request:
+ paths:
+ - .github/workflows/python-metadata-compatibility.yml
+ - superset/db_engine_specs/**
+ - tests/unit_tests/db_engine_specs/test_lint_metadata.py
+ - pyproject.toml
+ - superset/result_set.py
+ - superset/common/utils/dataframe_utils.py
+ - tests/unit_tests/common/test_dataframe_utils.py
+ - superset/utils/pandas_postprocessing/**
+ - tests/unit_tests/pandas_postprocessing/**
+ - tests/unit_tests/result_set_test.py
+ - superset/commands/importers/v1/utils.py
+ - superset/commands/dataset/importers/v1/utils.py
+ - tests/unit_tests/commands/importers/v1/utils_test.py
+ - superset/models/helpers.py
+ - tests/unit_tests/models/helpers_test.py
+ - superset/superset_typing.py
+ - tests/unit_tests/db_engine_specs/test_base.py
+ - superset/extensions/metadb.py
+ - tests/unit_tests/extensions/test_sqlalchemy.py
+ - tests/unit_tests/conftest.py
+ - Dockerfile
+ - docker/**
+ - docker-compose-image-tag.yml
+ - scripts/docker_release_target.py
+ - tests/unit_tests/docker_app_dependencies_test.py
+ - tests/unit_tests/docker_compose_image_tag_test.py
+ - tests/unit_tests/scripts/docker_release_target_test.py
+ - superset-core/**
+ workflow_dispatch: {}
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number ||
github.run_id }}
+ cancel-in-progress: true
+
+jobs:
+ metadata-parser:
+ runs-on: ubuntu-26.04
+ timeout-minutes: 10
+ strategy:
+ fail-fast: false
+ matrix:
+ python-version: [ "3.11", "3.12", "3.13", "3.14", "3.15" ]
+ env:
+ PYTEST_DISABLE_PLUGIN_AUTOLOAD: "1"
+ steps:
+ - name: Checkout
+ uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ with:
+ persist-credentials: false
+ - name: Setup Python
+ uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 #
v7.0.0
+ with:
+ python-version: ${{ matrix.python-version }}
+ allow-prereleases: true
+ - name: Install standalone test dependencies
+ run: python -m pip install pytest==7.4.4
+ - name: Test metadata parser without application dependencies
+ run: python -m pytest --noconftest -o addopts=
tests/unit_tests/db_engine_specs/test_lint_metadata.py -q -W
default::DeprecationWarning
+
+ runtime-compatibility:
+ runs-on: ubuntu-26.04
+ timeout-minutes: 25
+ strategy:
+ fail-fast: false
+ matrix:
+ python-version: [ "3.11", "3.12", "3.13", "3.14", "3.15" ]
+ env:
+ SUPERSET_SECRET_KEY: python-compatibility-ci-only-not-for-production
+ FLASK_APP: superset
+ steps:
+ - name: Checkout
+ uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ with:
+ persist-credentials: false
+ - name: Setup Python
+ uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 #
v7.0.0
+ with:
+ python-version: ${{ matrix.python-version }}
+ - name: Install application and focused test dependencies
+ run: >-
+ python -m pip install -e ./superset-core -e . pytest==7.4.4
pytest-mock freezegun parameterized responses rich prophet
+ - name: Check dependency consistency
+ run: python -m pip check
+ - name: Configure metadata directory
+ run: echo "SUPERSET_HOME=$RUNNER_TEMP/superset-python-compatibility" >>
"$GITHUB_ENV"
+ - name: Migrate a fresh metadata database
+ run: python -m flask db upgrade
+ - name: Test application health and runtime compatibility
+ run: |
+ python - <<'PY'
+ import pytest
+ from superset.app import create_app
+
+ app = create_app()
+ assert app.test_client().get('/health').status_code == 200
+ with app.app_context():
+ raise SystemExit(pytest.main([
+ '--confcutdir=tests/unit_tests', '-p', 'pytest_mock',
+ '-o', 'addopts=',
+ 'tests/unit_tests/db_engine_specs/test_lint_metadata.py',
+ 'tests/unit_tests/pandas_postprocessing',
+ 'tests/unit_tests/result_set_test.py',
+ 'tests/unit_tests/common/test_dataframe_utils.py',
+ 'tests/unit_tests/commands/importers/v1/utils_test.py',
+ 'tests/unit_tests/models/helpers_test.py',
Review Comment:
`tests/unit_tests/utils/test_date_parsing.py` is not in this pytest target
list, so the new mixed-offset regression for `normalize_dttm_col` (the Berlin
repeated-hour case, where both timestamps must normalise to local 02:30 without
raising) never runs on the Python 3.15 / pandas 3.1 combination this job exists
to cover. Regular unit CI runs on 3.11 and cannot exercise it. Could that file
be added here and to the `paths:` triggers?
##########
docs/admin_docs/installation/pypi.mdx:
##########
@@ -15,6 +15,75 @@ import useBaseUrl from '@docusaurus/useBaseUrl';
This page describes how to install Superset using the `apache_superset`
package [published on PyPI](https://pypi.org/project/apache_superset/).
+## Python Compatibility
+
+The minimum Python version is 3.11. The Python version classifiers in
+[pyproject.toml](https://github.com/apache/superset/blob/master/pyproject.toml)
+list the officially supported versions; the open-ended `requires-python` lower
+bound does not certify every newer interpreter.
+
+The source checkout includes experimental Python 3.15 compatibility. Its
package
+requirements select NumPy `>=2.5.4,<2.6` and pandas `>=3.1.0,<3.2` on Python
3.15
+and newer. Python 3.11 through 3.14 retain NumPy `>=1.23.5,<2.5` and pandas
+`>=2.3.3,<2.4`; the minimum Python version has not changed. These source
changes
Review Comment:
This says Python 3.11-3.14 keep pandas `>=2.3.3,<2.4`, but `pyproject.toml`
now requires `pandas[excel]>=3.0.6,<3.1` for `python_version < '3.15'`, and the
generated requirements pin 3.0.6. Someone constraining pandas to the documented
range gets an unsatisfiable install. Can this read `>=3.0.6,<3.1`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
