fix(date): import pandas locally in make_ts_exclusive#5885
Open
anxkhn wants to merge 1 commit into
Open
Conversation
make_ts_exclusive uses pd.Timedelta in its tsql branch, but pandas is only imported under TYPE_CHECKING (pandas is banned as a module-level import). Unlike its siblings make_inclusive_end and to_utc_timestamp, it never imports pandas at runtime, so make_ts_exclusive(time, dialect="tsql") raises NameError: name 'pd' is not defined. This is reachable when rendering an INCREMENTAL_BY_TIME_RANGE model with a jinja source/ref event-time filter on the tsql dialect (renderer.py), which surfaces as a ConfigError that the model could not be rendered. Add a local `import pandas as pd` inside make_ts_exclusive, mirroring the existing runtime-pandas helpers, and add a regression test covering both the tsql and non-tsql paths. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Contributor
Author
|
The only failing check here is
Could a maintainer re-run the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
make_ts_exclusiveusespd.Timedeltain itsdialect == "tsql"branch, butpandasis only imported underTYPE_CHECKINGinsqlmesh/utils/date.py(it isbanned as a module-level import by the ruff
flake8-tidy-importsconfig). Unlikethe sibling helpers
make_inclusive_endandto_utc_timestamp, which doimport pandas as pdat runtime,make_ts_exclusivenever imports pandas, so:This is reachable in normal use: rendering an
INCREMENTAL_BY_TIME_RANGEmodelthat has a jinja source/ref event-time filter on the
tsqldialect callsmake_ts_exclusive(..., dialect="tsql")insqlmesh/core/renderer.py. TheNameErroris caught there and re-raised as aConfigErrorreporting that themodel could not be rendered.
The fix adds a function-local
import pandas as pdas the first line ofmake_ts_exclusive, mirroring the existing runtime-pandas helpers in the samemodule. The non-tsql path is unaffected (it did not reference
pd).Test Plan
test_make_ts_exclusiveintests/utils/test_date.pycovering bothpaths: the
tsqldialect returnsto_utc_timestamp(ts) - 1ns(and no longerraises
NameError), and a non-tsql dialect (duckdb) returnsts + 1us.fails with
NameError: name 'pd' is not defined; with the fix it passes.pytest tests/utils/test_date.py-> 61 passed.pytest tests/utils/ -m "not slow and not docker"-> 236 passed, 1 skipped.ruff check+ruff format --check+mypyon the changed files -> clean.Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO