ronantakizawa/codereview-bench huggingface dataset:
Repository: encode/httpx
File: httpx/_client.py (Python)

Code before:
        Send a `DELETE` request.

        **Parameters**: See `httpx.request`.
        """
        return await self.request(
            "DELETE",
            url,
            params=params,
            headers=headers,
            cookies=cookies,
            auth=auth,
            allow_redirects=allow_redirects,
            timeout=timeout,
        )

    @check_not_closed
    async def aclose(self) -> None:
        """
        Close transport and proxies.
        """
        await self._transport.aclose()
        for proxy in self._proxies.values():
            if proxy is not None:
                await proxy.aclose()

        self._is_closed = True

    async def __aenter__(self) -> "AsyncClient":
        return self

    async def __aexit__(
        self,
        exc_type: typing.Type[BaseException] = None,
        exc_value: BaseException = None,
        traceback: TracebackType = None,
    ) -> None:
        await self.aclose()


class StreamContextManager:
    def __init__(
        self,
        client: BaseClient,
        request: Request,
        *,
        auth: AuthTypes = None,
        allow_redirects: bool = True,
        timeout: typing.Union[TimeoutTypes, UnsetType] = UNSET,
        close_client: bool = False,
    ) -> None:
        self.client = client

Review comment (bug):
We should put `self._is_closed = True` as the very first line, rather than the very last line, since that'd handle potential race conditions more cleanly.

Code after:
            timeout=timeout,
        )

    async def delete(
        self,
        url: URLTypes,
        *,
        params: QueryParamTypes = None,
        headers: HeaderTypes = None,
        cookies: CookieTypes = None,
        auth: typing.Union[AuthTypes, UnsetType] = UNSET,
        allow_redirects: bool = True,
        timeout: typing.Union[TimeoutTypes, UnsetType] = UNSET,
    ) -> Response:
        """
        Send a `DELETE` request.

        **Parameters**: See `httpx.request`.
        """
        return await self.request(
            "DELETE",
            url,
            params=params,
            headers=headers,
            cookies=cookies,
            auth=auth,
            allow_redirects=allow_redirects,
            timeout=timeout,
        )

    async def aclose(self) -> None:
        """
        Close transport and proxies.
        """
        if not self.is_closed:
            self._is_closed = True

            await self._transport.aclose()
            for proxy in self._proxies.values():
                if proxy is not None:
                    await proxy.aclose()

    async def __aenter__(self) -> "AsyncClient":
        await self._transport.__aenter__()
        for proxy in self._proxies.values():
            if proxy is not None:
                await proxy.__aenter__()
        self._is_closed = False
        return self

    async def __aexit__(
Repository: encode/httpx
File: httpx/_models.py (Python)

Code before:
        if stream is not None:
            self._raw_stream = stream
        else:
            self._raw_stream = ByteStream(body=content or b"")
            self.read()

    @property
    def elapsed(self) -> datetime.timedelta:
        """
        Returns the time taken for the complete request/response
        cycle to complete.
        """
        if not hasattr(self, "_elapsed"):
            raise RuntimeError(
                "'.elapsed' may only be accessed after the response "
                "has been read or closed."
            )
        return self._elapsed

    @property
    def request(self) -> Request:
        """
        Returns the request instance associated to the current response.
        """
        if self._request is None:
            raise RuntimeError("'.request' may only be accessed if initialized")
        return self._request

    @request.setter
    def request(self, value: Request) -> None:
        self._request = value

    @property
    def reason_phrase(self) -> str:
        return codes.get_reason_phrase(self.status_code)

    @property
    def url(self) -> typing.Optional[URL]:
        """
        Returns the URL for which the request was made.
        """
        return self.request.url

    @property
    def content(self) -> bytes:
        if not hasattr(self, "_content"):
            raise ResponseNotRead()
        return self._content

    @property
    def text(self) -> str:

Review comment (suggestion):
```suggestion
            raise RuntimeError("The request instance has not been set on this response.")
```

Code after:
        self.is_stream_consumed = False
        if stream is not None:
            self._raw_stream = stream
        else:
            self._raw_stream = ByteStream(body=content or b"")
            self.read()

    @property
    def elapsed(self) -> datetime.timedelta:
        """
        Returns the time taken for the complete request/response
        cycle to complete.
        """
        if not hasattr(self, "_elapsed"):
            raise RuntimeError(
                "'.elapsed' may only be accessed after the response "
                "has been read or closed."
            )
        return self._elapsed

    @property
    def request(self) -> Request:
        """
        Returns the request instance associated to the current response.
        """
        if self._request is None:
            raise RuntimeError(
                "The request instance has not been set on this response."
            )
        return self._request

    @request.setter
    def request(self, value: Request) -> None:
        self._request = value

    @property
    def reason_phrase(self) -> str:
        return codes.get_reason_phrase(self.status_code)

    @property
    def url(self) -> typing.Optional[URL]:
        """
        Returns the URL for which the request was made.
        """
        return self.request.url

    @property
    def content(self) -> bytes:
        if not hasattr(self, "_content"):
            raise ResponseNotRead()
        return self._content
Repository: encode/httpx
File: tests/client/test_event_hooks.py (Python)

Code before:
import pytest

import httpx


def test_event_hooks(server):
    events = []

    def on_request(request):
        events.append({"event": "request", "headers": dict(request.headers)})

    def on_response(response):
        headers = dict(response.headers)
        headers["date"] = "Mon, 24 Aug 2020 13:18:40 GMT"
        events.append({"event": "response", "headers": headers})

    event_hooks = {"request": [on_request], "response": [on_response]}

    with httpx.Client(event_hooks=event_hooks) as http:
        http.get(server.url, auth=("username", "password"))

    assert events == [
        {
            "event": "request",
            "headers": {
                "host": "127.0.0.1:8000",
                "user-agent": "python-httpx/0.14.2",
                "accept": "*/*",
                "accept-encoding": "gzip, deflate, br",
                "connection": "keep-alive",
                "authorization": "Basic dXNlcm5hbWU6cGFzc3dvcmQ=",

Review comment (security):
It seems we're not testing for "request hooks are called on redirect requests", which makes me think the decision to put request hooks at the top of `._send_handling_auth()` was not entirely deliberate? :-)

Any case, whichever we decide I guess we'd need to add explicit tests for what we expect to happen as requests and responses go through the send-redirects-auth-(retries soon) stack?

Code after:
import pytest

import httpx
from tests.utils import AsyncMockTransport, MockTransport


def app(request: httpx.Request) -> httpx.Response:
    if request.url.path == "/redirect":
        return httpx.Response(303, headers={"server": "testserver", "location": "/"})
    elif request.url.path.startswith("/status/"):
        status_code = int(request.url.path[-3:])
        return httpx.Response(status_code, headers={"server": "testserver"})

    return httpx.Response(200, headers={"server": "testserver"})


def test_event_hooks():
    events = []

    def on_request(request):
        events.append({"event": "request", "headers": dict(request.headers)})

    def on_response(response):
        events.append({"event": "response", "headers": dict(response.headers)})

    event_hooks = {"request": [on_request], "response": [on_response]}

    with httpx.Client(event_hooks=event_hooks, transport=MockTransport(app)) as http:
        http.get("http://127.0.0.1:8000/", auth=("username", "password"))

    assert events == [
Repository: encode/httpx
File: httpx/_content_streams.py (Python)

Code before:
    files: RequestFiles = None,
    json: typing.Any = None,
    boundary: bytes = None,
) -> ContentStream:
    """
    Handles encoding the given `data`, `files`, and `json`, returning
    a `ContentStream` implementation.
    """
    if not data:
        if json is not None:
            return JSONStream(json=json)
        elif files:
            return MultipartStream(data={}, files=files, boundary=boundary)
        else:
            return ByteStream(body=b"")
    elif isinstance(data, dict):
        if files:
            return MultipartStream(data=data, files=files, boundary=boundary)
        else:
            return URLEncodedStream(data=data)
    elif isinstance(data, (str, bytes)):
        return ByteStream(body=data)
    elif hasattr(data, "__aiter__"):
        data = typing.cast(typing.AsyncIterator[bytes], data)
        return AsyncIteratorStream(aiterator=data)
    elif hasattr(data, "__iter__"):
        data = typing.cast(typing.Iterator[bytes], data)
        return IteratorStream(iterator=data)

    raise TypeError(f"Unexpected type for 'data', {type(data)!r}")


Review comment (suggestion):
Maybe you were already aware, but a thing I just learned from reviewing encode/starlette#1041: `collections.abc.AsyncIterator`/`collections.abc.Iterator` (which the `typing` equivalents derive from) have [a subclasshook](https://github.com/python/cpython/blob/v3.8.5/Lib/_collections_abc.py#L185-L189) which I think means you could just do:
```python
    elif isinstance(content, typing.AsyncIterator):
          return AsyncIteratorStream(aiterator=content)
```

Also _technically_ you may be casting Iterables to _Iterators_ here by just checking for the presence of `__aiter__`, though it seems unlikely to cause problems.

Code after:
            return ByteStream(body=b"")
    elif isinstance(data, dict):
        if files:
            return MultipartStream(data=data, files=files, boundary=boundary)
        else:
            return URLEncodedStream(data=data)
    elif isinstance(data, (str, bytes)):
        return ByteStream(body=data)
    elif hasattr(data, "__aiter__"):
        data = typing.cast(typing.AsyncIterator[bytes], data)
        return AsyncIteratorStream(aiterator=data)
    elif hasattr(data, "__iter__"):
        data = typing.cast(typing.Iterator[bytes], data)
        return IteratorStream(iterator=data)

    raise TypeError(f"Unexpected type for 'data', {type(data)!r}")


def encode_response(content: ResponseContent = None) -> ContentStream:
    if content is None:
        return ByteStream(b"")
    elif isinstance(content, bytes):
        return ByteStream(body=content)
    elif hasattr(content, "__aiter__"):
        content = typing.cast(typing.AsyncIterator[bytes], content)
        return AsyncIteratorStream(aiterator=content)
    elif hasattr(content, "__iter__"):
        content = typing.cast(typing.Iterator[bytes], content)
        return IteratorStream(iterator=content)

    raise TypeError(f"Unexpected type for 'content', {type(content)!r}")
Repository: encode/httpx
File: tests/models/test_url.py (Python)

Code before:
    assert url.raw_path == b"/path"

    url = httpx.URL("https://www.example.com/path?")
    assert url.path == "/path"
    assert url.query == b""
    assert url.raw_path == b"/path?"


def test_url_with_url_encoded_path():
    url = httpx.URL("https://www.example.com/path%20to%20somewhere")
    assert url.path == "/path to somewhere"
    assert url.query == b""
    assert url.raw_path == b"/path%20to%20somewhere"


def test_ipv6_url():
    url = httpx.URL("http://[::ffff:192.168.0.1]:5678/")

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:5678"


@pytest.mark.parametrize(
    "url_str",
    [
        "http://192.168.0.1:1234",
        "http://example.com:1234",
        "http://[::ffff:192.168.0.2]:1234",
    ],
)
@pytest.mark.parametrize("new_host", ["[::ffff:192.168.0.1]", "::ffff:192.168.0.1"])
def test_ipv6_url_copy_with_host(url_str, new_host):
    url = httpx.URL(url_str).copy_with(host=new_host)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:1234"
    assert str(url) == "http://[::ffff:192.168.0.1]:1234"


@pytest.mark.parametrize("host", [b"[::ffff:192.168.0.1]", b"::ffff:192.168.0.1"])
def test_ipv6_url_from_raw_url(host):
    raw_url = (b"https", host, 443, b"/")
    url = httpx.URL(raw_url)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:443"
    assert str(url) == "https://[::ffff:192.168.0.1]:443/"


Review comment (suggestion):
The test with this parameter doesn't make sense.
```suggestion
        "http://127.0.0.1:1234",
```

Code after:
    assert url.raw_path == b"/path"

    url = httpx.URL("https://www.example.com/path?")
    assert url.path == "/path"
    assert url.query == b""
    assert url.raw_path == b"/path?"


def test_url_with_url_encoded_path():
    url = httpx.URL("https://www.example.com/path%20to%20somewhere")
    assert url.path == "/path to somewhere"
    assert url.query == b""
    assert url.raw_path == b"/path%20to%20somewhere"


def test_ipv6_url():
    url = httpx.URL("http://[::ffff:192.168.0.1]:5678/")

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:5678"


@pytest.mark.parametrize(
    "url_str",
    [
        "http://127.0.0.1:1234",
        "http://example.com:1234",
        "http://[::ffff:127.0.0.1]:1234",
    ],
)
@pytest.mark.parametrize("new_host", ["[::ffff:192.168.0.1]", "::ffff:192.168.0.1"])
def test_ipv6_url_copy_with_host(url_str, new_host):
    url = httpx.URL(url_str).copy_with(host=new_host)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:1234"
    assert str(url) == "http://[::ffff:192.168.0.1]:1234"


@pytest.mark.parametrize("host", [b"[::ffff:192.168.0.1]", b"::ffff:192.168.0.1"])
def test_ipv6_url_from_raw_url(host):
    raw_url = (b"https", host, 443, b"/")
    url = httpx.URL(raw_url)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:443"
    assert str(url) == "https://[::ffff:192.168.0.1]:443/"
Repository: encode/httpx
File: tests/models/test_url.py (Python)

Code before:
    url = httpx.URL("https://www.example.com/path?")
    assert url.path == "/path"
    assert url.query == b""
    assert url.raw_path == b"/path?"


def test_url_with_url_encoded_path():
    url = httpx.URL("https://www.example.com/path%20to%20somewhere")
    assert url.path == "/path to somewhere"
    assert url.query == b""
    assert url.raw_path == b"/path%20to%20somewhere"


def test_ipv6_url():
    url = httpx.URL("http://[::ffff:192.168.0.1]:5678/")

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:5678"


@pytest.mark.parametrize(
    "url_str",
    [
        "http://127.0.0.1:1234",
        "http://example.com:1234",
        "http://[::ffff:192.168.0.2]:1234",
    ],
)
@pytest.mark.parametrize("new_host", ["[::ffff:192.168.0.1]", "::ffff:192.168.0.1"])
def test_ipv6_url_copy_with_host(url_str, new_host):
    url = httpx.URL(url_str).copy_with(host=new_host)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:1234"
    assert str(url) == "http://[::ffff:192.168.0.1]:1234"


@pytest.mark.parametrize("host", [b"[::ffff:192.168.0.1]", b"::ffff:192.168.0.1"])
def test_ipv6_url_from_raw_url(host):
    raw_url = (b"https", host, 443, b"/")
    url = httpx.URL(raw_url)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:443"
    assert str(url) == "https://[::ffff:192.168.0.1]:443/"


Review comment (suggestion):
Change this host as the above
```suggestion
        "http://[::ffff:127.0.0.1]:1234",
```

Code after:
    url = httpx.URL("https://www.example.com/path?")
    assert url.path == "/path"
    assert url.query == b""
    assert url.raw_path == b"/path?"


def test_url_with_url_encoded_path():
    url = httpx.URL("https://www.example.com/path%20to%20somewhere")
    assert url.path == "/path to somewhere"
    assert url.query == b""
    assert url.raw_path == b"/path%20to%20somewhere"


def test_ipv6_url():
    url = httpx.URL("http://[::ffff:192.168.0.1]:5678/")

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:5678"


@pytest.mark.parametrize(
    "url_str",
    [
        "http://127.0.0.1:1234",
        "http://example.com:1234",
        "http://[::ffff:127.0.0.1]:1234",
    ],
)
@pytest.mark.parametrize("new_host", ["[::ffff:192.168.0.1]", "::ffff:192.168.0.1"])
def test_ipv6_url_copy_with_host(url_str, new_host):
    url = httpx.URL(url_str).copy_with(host=new_host)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:1234"
    assert str(url) == "http://[::ffff:192.168.0.1]:1234"


@pytest.mark.parametrize("host", [b"[::ffff:192.168.0.1]", b"::ffff:192.168.0.1"])
def test_ipv6_url_from_raw_url(host):
    raw_url = (b"https", host, 443, b"/")
    url = httpx.URL(raw_url)

    assert url.host == "::ffff:192.168.0.1"
    assert url.netloc == "[::ffff:192.168.0.1]:443"
    assert str(url) == "https://[::ffff:192.168.0.1]:443/"
Repository: encode/httpx
File: httpx/_multipart.py (Python)

Code before:
import binascii
import os
import typing
from pathlib import Path

from ._types import FileContent, FileTypes, RequestFiles
from ._utils import (
    format_form_param,
    guess_content_type,
    peek_filelike_length,
    to_bytes,
)


class DataField:
    """
    A single form field item, within a multipart form field.
    """

    def __init__(self, name: str, value: typing.Union[str, bytes]) -> None:
        if not isinstance(name, str):
            raise TypeError(f"Invalid type for name. Expected str, "
                            f"got {type(name).__name__}: {name!r}")
        if not isinstance(value, (str, bytes)):
            raise TypeError(f"Invalid type for value. Expected str or bytes, "
                            f"got {type(value).__name__}: {value!r}")
        self.name = name
        self.value = value

    def render_headers(self) -> bytes:
        if not hasattr(self, "_headers"):
            name = format_form_param("name", self.name)
            self._headers = b"".join(
                [b"Content-Disposition: form-data; ", name, b"\r\n\r\n"]
            )

        return self._headers

    def render_data(self) -> bytes:
        if not hasattr(self, "_data"):
            self._data = (
                self.value
                if isinstance(self.value, bytes)
                else self.value.encode("utf-8")
            )

        return self._data


Review comment (suggestion):
```suggestion
            raise TypeError(
                f"Invalid type for name. Expected str, got {type(name)}: {name!r}"
            )
```

Code after:
import binascii
import os
import typing
from pathlib import Path

from ._types import FileContent, FileTypes, RequestFiles
from ._utils import (
    format_form_param,
    guess_content_type,
    peek_filelike_length,
    to_bytes,
)


class DataField:
    """
    A single form field item, within a multipart form field.
    """

    def __init__(self, name: str, value: typing.Union[str, bytes]) -> None:
        if not isinstance(name, str):
            raise TypeError(
                f"Invalid type for name. Expected str, got {type(name)}: {name!r}"
            )
        if not isinstance(value, (str, bytes)):
            raise TypeError(
                f"Invalid type for value. Expected str or bytes, got {type(value)}: {value!r}"
            )
        self.name = name
        self.value = value

    def render_headers(self) -> bytes:
        if not hasattr(self, "_headers"):
            name = format_form_param("name", self.name)
            self._headers = b"".join(
                [b"Content-Disposition: form-data; ", name, b"\r\n\r\n"]
            )

        return self._headers

    def render_data(self) -> bytes:
        if not hasattr(self, "_data"):
            self._data = (
                self.value
                if isinstance(self.value, bytes)
                else self.value.encode("utf-8")
            )
Repository: encode/httpx
File: httpx/_multipart.py (Python)

Code before:
import binascii
import os
import typing
from pathlib import Path

from ._types import FileContent, FileTypes, RequestFiles
from ._utils import (
    format_form_param,
    guess_content_type,
    peek_filelike_length,
    to_bytes,
)


class DataField:
    """
    A single form field item, within a multipart form field.
    """

    def __init__(self, name: str, value: typing.Union[str, bytes]) -> None:
        if not isinstance(name, str):
            raise TypeError(f"Invalid type for name. Expected str, "
                            f"got {type(name).__name__}: {name!r}")
        if not isinstance(value, (str, bytes)):
            raise TypeError(f"Invalid type for value. Expected str or bytes, "
                            f"got {type(value).__name__}: {value!r}")
        self.name = name
        self.value = value

    def render_headers(self) -> bytes:
        if not hasattr(self, "_headers"):
            name = format_form_param("name", self.name)
            self._headers = b"".join(
                [b"Content-Disposition: form-data; ", name, b"\r\n\r\n"]
            )

        return self._headers

    def render_data(self) -> bytes:
        if not hasattr(self, "_data"):
            self._data = (
                self.value
                if isinstance(self.value, bytes)
                else self.value.encode("utf-8")
            )

        return self._data

    def get_length(self) -> int:
        headers = self.render_headers()
        data = self.render_data()

Review comment (suggestion):
```suggestion
            raise TypeError(
                f"Invalid type for value. Expected str or bytes, got {type(value)}: {value!r}"
            )
```

Code after:
import binascii
import os
import typing
from pathlib import Path

from ._types import FileContent, FileTypes, RequestFiles
from ._utils import (
    format_form_param,
    guess_content_type,
    peek_filelike_length,
    to_bytes,
)


class DataField:
    """
    A single form field item, within a multipart form field.
    """

    def __init__(self, name: str, value: typing.Union[str, bytes]) -> None:
        if not isinstance(name, str):
            raise TypeError(
                f"Invalid type for name. Expected str, got {type(name)}: {name!r}"
            )
        if not isinstance(value, (str, bytes)):
            raise TypeError(
                f"Invalid type for value. Expected str or bytes, got {type(value)}: {value!r}"
            )
        self.name = name
        self.value = value

    def render_headers(self) -> bytes:
        if not hasattr(self, "_headers"):
            name = format_form_param("name", self.name)
            self._headers = b"".join(
                [b"Content-Disposition: form-data; ", name, b"\r\n\r\n"]
            )

        return self._headers

    def render_data(self) -> bytes:
        if not hasattr(self, "_data"):
            self._data = (
                self.value
                if isinstance(self.value, bytes)
                else self.value.encode("utf-8")
            )

        return self._data

    def get_length(self) -> int: