Skip to content

Remove SQLModel query deprecation warnings#693

Open
MaxGhenis wants to merge 1 commit intomainfrom
codex/fix-ci-warnings
Open

Remove SQLModel query deprecation warnings#693
MaxGhenis wants to merge 1 commit intomainfrom
codex/fix-ci-warnings

Conversation

@MaxGhenis
Copy link
Copy Markdown
Contributor

Summary

  • replace legacy session.query(...) lookups with session.exec(select(...)) in the SOI ETL loaders
  • update the focused national-target test to use the same SQLModel pattern and avoid test-side warnings
  • add a changelog fragment for the CI warning cleanup

Testing

  • UV_PYTHON=3.14 uv run pytest tests/unit/test_etl_irs_soi_overlay.py tests/unit/test_etl_national_targets.py -q
  • UV_PYTHON=3.14 uv run ruff check policyengine_us_data/db/etl_irs_soi.py policyengine_us_data/db/etl_national_targets.py tests/unit/test_etl_national_targets.py
  • UV_PYTHON=3.14 uv run ruff format --check policyengine_us_data/db/etl_irs_soi.py policyengine_us_data/db/etl_national_targets.py tests/unit/test_etl_national_targets.py

Copy link
Copy Markdown
Collaborator

@baogorek baogorek left a comment

Choose a reason for hiding this comment

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

Mostly a "safe, mechanical refactor," but I'm requesting one line be removed (with .unique). Let me know if you want to discuss.

),
)
)
.unique()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code smell for me. I really don't like deduplication commands unless there's a very good reason for them. From Claude:

  The thing is — this query has no .join(). It's a simple select(Stratum).where(...) with an .in_() filter.   
  There shouldn't be duplicate rows from that. The .unique() is unnecessary here. 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants