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

Initial work to enable developing Sematic with uv #1136

Merged
merged 5 commits into from
Dec 12, 2024
Merged

Initial work to enable developing Sematic with uv #1136

merged 5 commits into from
Dec 12, 2024

Conversation

augray
Copy link
Member

@augray augray commented Dec 11, 2024

As part of #1135, we need an alternative way to manage python dependencies besides bazel. This PR introduces the foundation required to use uv to serve this purpose. Introducing uv at python3.12 (a python version currently untested via bazel) necessitated upgrading some packages, including linting tools, so there are changes which may not directly appear to be related to packaging. However, all changes in this PR should not impact logic (beyond test logic, usually changes to this are noted).

This PR is the first in an expected series of PRs.

@@ -41,9 +41,6 @@ commands:
- run:
name: Black
command: black sematic --diff --check
- run:
Copy link
Member Author

Choose a reason for hiding this comment

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

isort and black were wanting to make conflicting changes to the code. As part of the overall modernization of dependencies, I would like to use ruff for import ordering, formatting and linting. This will replace isort, flake8, black with a single tool designed to handle all of these things without conflicts like this (and much faster as well). For now, we'll just ignore the isort check in CI, knowing that imports are well ordered now and will be again soon once we get the tooling itself sorted out.

pyproject.toml Outdated
[project.optional-dependencies]
examples = [
# Examples
# "snowflake-connector-python==3.0.4",
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm working on getting the commented out example deps uncommented.

pyproject.toml Outdated

[tool.pytest.ini_options]
addopts = [
"--ignore=sematic/api/endpoints/tests/test_artifacts.py",
Copy link
Member Author

Choose a reason for hiding this comment

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

Working on getting these tests passing again. Most are failing due to import issues with dependencies from the examples extra.

@@ -1,6 +1,7 @@
pytest_test(
name = "test_external_resources",
srcs = ["test_external_resources.py"],
py_versions = [PY3.PY3_9, PY3.PY3_10, PY3.PY3_11],
Copy link
Member Author

@augray augray Dec 11, 2024

Choose a reason for hiding this comment

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

Just removing testing of python3.8 from certain tests that are giving some trouble with python3.8, not because of issues with the code they are testing, but rather with the tests themselves.

Specifically, they use parenthetical context manager statements that are supported in 3.9 and above but not 3.8. This hadn't been a problem because the formatter wasn't forcing this syntax until this upgrade.

But since python3.8 is EOL and we are about to drop support, easier to just not run these specific tests with 3.8 for now.

@@ -108,18 +108,6 @@ def test_run_state_changed(persisted_run: Run): # noqa: F811
"columns": ["function_path"],
},
),
(
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests are non-deterministic depending on the platform and timezone settings.

@augray augray requested a review from neutralino1 December 12, 2024 14:50
@augray augray changed the title [DRAFT] Initial work to enable developing Sematic with uv Initial work to enable developing Sematic with uv Dec 12, 2024
Copy link
Member

@neutralino1 neutralino1 left a comment

Choose a reason for hiding this comment

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

Thanks for this work 👍

@@ -23,13 +23,23 @@ pre-commit:
python3 -m flake8
python3 -m mypy sematic
python3 -m black sematic --check
Copy link
Member

Choose a reason for hiding this comment

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

Not removing black too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will; I'm going to get things working in parallel before removing the old tooling

Comment on lines +4 to +7
flake8==7.1.1
mypy==1.13.0
black==24.10.0
isort==5.13.2
Copy link
Member

Choose a reason for hiding this comment

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

Still needed after introducing ruff?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll remove them once I get stuff working with the uv toolchain in CI; coming in a separate PR

@augray augray added this pull request to the merge queue Dec 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2024
@augray augray added this pull request to the merge queue Dec 12, 2024
Merged via the queue into main with commit d87d47a Dec 12, 2024
8 checks passed
@augray augray deleted the augray/uv branch December 12, 2024 22:00
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