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

Add Python formatting with Ruff #2573

Merged
merged 3 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@

# Apply clang-format to the project.
5e8537a77e760c160ace3dfe23ee8c76ee5aeeb3

# Apply ruff format to the project
d6d0607a845e6f71084ce272a1c1e8c50e244bdd
6 changes: 6 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ jobs:
- uses: actions/checkout@v4
with:
show-progress: false
- uses: actions/setup-python@v5
with:
python-version: 3.12
Copy link
Member

Choose a reason for hiding this comment

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

Can we have a .python-version file in the root of this project, so we don't have to use python-version: CONSTANT in everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I only see 3.12 repeated in two places, so I'm not sure this would be helpful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW the Ubuntu 24.04 dev image defaults to python 3.12, so this is currently a no-op (https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md#language-and-runtime). There's a case for keeping it though to use a fixed python version even if we update the build image, or if we want to update to python 3.13 early (comes out in 6 weeks).

Copy link
Member

Choose a reason for hiding this comment

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

This is also an indicator for IDEs to automatically use a specific version of Python whenever the interpreter is not configured. I wish this change was included in this pull-request.

- name: Setup Linux
run: |
export DEBIAN_FRONTEND=noninteractive
Expand All @@ -27,6 +30,9 @@ jobs:
- name: Install project deps with pnpm
run: |
pnpm i
- name: Install Ruff
run: |
pip install ruff
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well with the current approach it works locally for people to do python ./tools/cross/format.py if they have ruff installed, whereas if we used the action there wouldn't be a good local approach. I think @danlapid said he'd prefer to modify format.py over using ci-specific automation.

Copy link
Member

Choose a reason for hiding this comment

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

ruff-action is used to install ruff. it is not used to execute ruff. right now, we will always use the latest ruff version.

- name: Lint
run: |
python3 ./tools/cross/format.py --check
Expand Down
1 change: 1 addition & 0 deletions samples/pyodide-fastapi/worker.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from asgi import env


async def on_fetch(request):
import asgi

Expand Down
13 changes: 8 additions & 5 deletions samples/pyodide-langchain/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@

API_KEY = "sk-abcdefg"


async def test(request):
prompt = PromptTemplate.from_template("Complete the following sentence: I am a {profession} and ")
llm = OpenAI(api_key=API_KEY)
chain = prompt | llm
prompt = PromptTemplate.from_template(
"Complete the following sentence: I am a {profession} and "
)
llm = OpenAI(api_key=API_KEY)
chain = prompt | llm

res = await chain.ainvoke({"profession": "electrician"})
print(res)
res = await chain.ainvoke({"profession": "electrician"})
print(res)
12 changes: 6 additions & 6 deletions samples/repl-server-python/worker.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

from js import Response

import io
Expand All @@ -12,12 +11,13 @@

ii = code.InteractiveInterpreter()


async def on_fetch(request, env):
cmd = (await request.json()).cmd
cmd = (await request.json()).cmd

ii.runsource(cmd)
ii.runsource(cmd)

res = sys.stdout.getvalue()
sys.stdout = StringIO()
res = sys.stdout.getvalue()
sys.stdout = StringIO()

return Response.new(res)
return Response.new(res)
9 changes: 5 additions & 4 deletions src/cloudflare/internal/test/ai/ai-api-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
# Licensed under the Apache 2.0 license found in the LICENSE file or at:
# https://opensource.org/licenses/Apache-2.0


async def test(context, env):
resp = await env.ai.run('testModel', {"prompt": 'test'})
assert resp.response == "model response"
resp = await env.ai.run("testModel", {"prompt": "test"})
assert resp.response == "model response"

# Test request id is present
assert env.ai.lastRequestId == '3a1983d7-1ddd-453a-ab75-c4358c91b582'
# Test request id is present
assert env.ai.lastRequestId == "3a1983d7-1ddd-453a-ab75-c4358c91b582"
46 changes: 24 additions & 22 deletions src/cloudflare/internal/test/d1/d1-api-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,38 @@
# Licensed under the Apache 2.0 license found in the LICENSE file or at:
# https://opensource.org/licenses/Apache-2.0


def assertSuccess(ret):
# D1 operations return an object with a `success` field.
assert ret.success
# D1 operations return an object with a `success` field.
assert ret.success


async def test(context, env):
DB = env.d1
DB = env.d1

assertSuccess(
await DB.prepare(
"""CREATE TABLE users
(
user_id INTEGER PRIMARY KEY,
name TEXT,
home TEXT,
features TEXT,
land_based BOOLEAN
);
"""
).run()
)
assertSuccess(
await DB.prepare(
"""CREATE TABLE users
(
user_id INTEGER PRIMARY KEY,
name TEXT,
home TEXT,
features TEXT,
land_based BOOLEAN
);
"""
).run()
)

result = await DB.prepare(
"""
result = await DB.prepare(
"""
INSERT INTO users (name, home, features, land_based) VALUES
('Albert Ross', 'sky', 'wingspan', false),
('Al Dente', 'bowl', 'mouthfeel', true)
RETURNING *
"""
"""
).all()

assertSuccess(result)
assert len(result.results) == 2
assert result.results[0].name == "Albert Ross"
assertSuccess(result)
assert len(result.results) == 2
assert result.results[0].name == "Albert Ross"
34 changes: 22 additions & 12 deletions src/cloudflare/internal/test/vectorize/vectorize-api-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,29 @@
from pyodide.ffi import to_js as _to_js
from js import Object


def to_js(obj):
return _to_js(obj, dict_converter=Object.fromEntries)
return _to_js(obj, dict_converter=Object.fromEntries)


async def test(context, env):
IDX = env.vectorSearch
IDX = env.vectorSearch

res_array = [0] * 5
# TODO(EW-8209): This shouldn't require `to_js`. It subtly fails without it.
results = await IDX.query(Float32Array.new(res_array), to_js({
"topK": 3,
"returnValues": True,
"returnMetadata": True,
}))
assert results.count > 0
assert results.matches[0].id == "b0daca4a-ffd8-4865-926b-e24800af2a2d"
assert results.matches[1].metadata.text == "Peter Piper picked a peck of pickled peppers"
res_array = [0] * 5
# TODO(EW-8209): This shouldn't require `to_js`. It subtly fails without it.
results = await IDX.query(
Float32Array.new(res_array),
to_js(
{
"topK": 3,
"returnValues": True,
"returnMetadata": True,
}
),
)
assert results.count > 0
assert results.matches[0].id == "b0daca4a-ffd8-4865-926b-e24800af2a2d"
assert (
results.matches[1].metadata.text
== "Peter Piper picked a peck of pickled peppers"
)
10 changes: 7 additions & 3 deletions src/pyodide/internal/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

ASGI = {"spec_version": "2.0", "version": "3.0"}


@Depends
async def env(request: Request):
return request.scope["env"]


@contextmanager
def acquire_js_buffer(pybuffer):
from pyodide.ffi import create_proxy
Expand Down Expand Up @@ -45,15 +47,17 @@ def request_to_scope(req, env, ws=False):
"path": path,
"query_string": query_string,
"type": ty,
"env": env
"env": env,
}


async def start_application(app):
shutdown_future = Future()

async def shutdown():
shutdown_future.set_result(None)
await sleep(0)

it = iter([{"type": "lifespan.startup"}, Future()])

async def receive():
Expand All @@ -65,11 +69,11 @@ async def receive():
ready = Future()

async def send(got):
if got['type'] == 'lifespan.startup.complete':
if got["type"] == "lifespan.startup.complete":
print("Application startup complete.")
print("Uvicorn running")
ready.set_result(None)
if got['type'] == 'lifespan.shutdown.complete':
if got["type"] == "lifespan.shutdown.complete":
print("Application shutdown complete")
raise RuntimeError(f"Unexpected lifespan event {got['type']}")

Expand Down
42 changes: 22 additions & 20 deletions src/pyodide/internal/patches/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
from typing import Any, Optional, Iterable
from yarl import URL


class Content:
__slots__ = ("_jsresp", "_exception")

def __init__(self, _jsresp):
self._jsresp = _jsresp
self._exception = None
Expand All @@ -30,34 +32,35 @@ def exception(self):
def set_exception(self, exc: BaseException) -> None:
self._exception = exc


async def _request(
self,
method: str,
str_or_url,
*,
params = None,
params=None,
data: Any = None,
json: Any = None,
cookies = None,
headers = None,
cookies=None,
headers=None,
skip_auto_headers: Optional[Iterable[str]] = None,
auth = None,
auth=None,
allow_redirects: bool = True,
max_redirects: int = 10,
compress: Optional[str] = None,
chunked: Optional[bool] = None,
expect100: bool = False,
raise_for_status = None,
raise_for_status=None,
read_until_eof: bool = True,
proxy = None,
proxy_auth = None,
timeout = None,
proxy=None,
proxy_auth=None,
timeout=None,
verify_ssl: Optional[bool] = None,
fingerprint: Optional[bytes] = None,
ssl_context = None,
ssl = None,
proxy_headers = None,
trace_request_ctx = None,
ssl_context=None,
ssl=None,
proxy_headers=None,
trace_request_ctx=None,
read_bufsize: Optional[int] = None,
):
# NOTE: timeout clamps existing connect and read timeouts. We cannot
Expand All @@ -70,13 +73,10 @@ async def _request(
ssl = _merge_ssl_params(ssl, verify_ssl, ssl_context, fingerprint)

if data is not None and json is not None:
raise ValueError(
"data and json parameters can not be used at the same time"
)
raise ValueError("data and json parameters can not be used at the same time")
elif json is not None:
data = payload.JsonPayload(json, dumps=self._json_serialize)


redirects = 0
history = []
version = self._version
Expand Down Expand Up @@ -125,8 +125,7 @@ async def _request(
url, auth_from_url = strip_auth_from_url(url)
if auth and auth_from_url:
raise ValueError(
"Cannot combine AUTH argument with "
"credentials encoded in URL"
"Cannot combine AUTH argument with credentials encoded in URL"
)

if auth is None:
Expand Down Expand Up @@ -194,13 +193,16 @@ async def _request(
)
from js import fetch, Headers
from pyodide.ffi import to_js

body = None
if req.body:
body = to_js(req.body._value)
jsheaders = Headers.new()
for k, v in headers.items():
jsheaders.append(k, v)
jsresp = await fetch(str(req.url), method=req.method, headers=jsheaders, body=body)
jsresp = await fetch(
str(req.url), method=req.method, headers=jsheaders, body=body
)
resp.version = version
resp.status = jsresp.status
resp.reason = jsresp.statusText
Expand All @@ -209,7 +211,6 @@ async def _request(
resp._raw_headers = tuple(tuple(e) for e in jsresp.headers)
resp.content = Content(jsresp)


# check response status
if raise_for_status is None:
raise_for_status = self._raise_for_status
Expand Down Expand Up @@ -249,4 +250,5 @@ async def _request(
)
raise


ClientSession._request = _request
4 changes: 3 additions & 1 deletion src/pyodide/internal/process_script_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
# All it does is walk through the imports in each of the worker's modules and attempts to import
# them. Local imports are not possible because the worker file path is explicitly removed from the
# module search path.
CF_LOADED_MODULES=[]
CF_LOADED_MODULES = []


def _do_it():
import ast
from pathlib import Path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@
import sys

RUST_PACKAGES = ["pydantic_core", "tiktoken"]
MODULES_TO_PATCH = ["random", "numpy.random", "numpy.random.mtrand", "tempfile", "aiohttp.http_websocket"] + RUST_PACKAGES
MODULES_TO_PATCH = [
"random",
"numpy.random",
"numpy.random.mtrand",
"tempfile",
"aiohttp.http_websocket",
] + RUST_PACKAGES

# Control number of allowed entropy calls.

Expand Down Expand Up @@ -136,6 +142,7 @@ def pydantic_core_context(module):
@contextmanager
def aiohttp_http_websocket_context(module):
import random

Random = random.Random

def patched_Random():
Expand Down
Loading
Loading