3
28
Fork
You've already forked pyforgejo
6

Just about everything is typing.Optional which makes writing type-aware code awkward #42

Open
opened 2025年12月12日 01:17:18 +01:00 by AdamWill · 0 comments

It's not easy for pyforgejo to do much about this, I don't think, but I figured it's at least worth noting.

I think because the forgejo API doesn't use required to indicate which response keywords we can rely upon to exist, pyforgejo winds up using typing.Optional for every property of every response object. This means that, technically, you can't rely on anything not being None, and type checkers will complain if you do, like this:

src/ai_code_review/core/forgejo_client.py:109: error: Argument "id" to "PullRequestInfo" has incompatible type "int | None"; expected "int" [arg-type]
src/ai_code_review/core/forgejo_client.py:110: error: Argument "number" to "PullRequestInfo" has incompatible type "int | None"; expected "int" [arg-type]
src/ai_code_review/core/forgejo_client.py:111: error: Argument "title" to "PullRequestInfo" has incompatible type "str | None"; expected "str" [arg-type]
src/ai_code_review/core/forgejo_client.py:113: error: Item "None" of "PrBranchInfo | None" has no attribute "ref" [union-attr]
src/ai_code_review/core/forgejo_client.py:113: error: Argument "source_branch" to "PullRequestInfo" has incompatible type "str | Any | None"; expected "str" [arg-type]
src/ai_code_review/core/forgejo_client.py:114: error: Item "None" of "PrBranchInfo | None" has no attribute "ref" [union-attr]
src/ai_code_review/core/forgejo_client.py:114: error: Argument "target_branch" to "PullRequestInfo" has incompatible type "str | Any | None"; expected "str" [arg-type]
src/ai_code_review/core/forgejo_client.py:115: error: Item "None" of "User | None" has no attribute "login" [union-attr]
src/ai_code_review/core/forgejo_client.py:115: error: Argument "author" to "PullRequestInfo" has incompatible type "str | Any | None"; expected "str" [arg-type]
src/ai_code_review/core/forgejo_client.py:116: error: Argument "state" to "PullRequestInfo" has incompatible type "str | None"; expected "str" [arg-type]
src/ai_code_review/core/forgejo_client.py:117: error: Argument "web_url" to "PullRequestInfo" has incompatible type "str | None"; expected "str" [arg-type]

these are all because the types in pyforgejo's PullRequest model (where I'm getting the values from) are typing.Optional[int], typing.Optional[str] etc. Making all your code (somewhat nonsensically) defend against things like the pull request number potentially being None is a bit of a drag.

Specifically I think this happens because there's no real way to indicate in a Python class that it may or may not have a given property with particular attributes, so instead fern gives properties that aren't required in the API definition a default value of None and makes them typing.Optional, but I'm guessing a bit there, fern is a huge codebase and it's hard to find the bit that does this.

It's also worth noting that forgejo's API definition does not indicate which response properties are nullable. Technically it's saying that none of them are nullable; a literal reading of the API definition is something like "you can't rely on any of these properties being in the response at all, but if they are, none of them will be null". Which is obviously not true. The real situation seems to be more like "you can always rely on all the properties being in the response, but some of them might be null".

It's not easy for pyforgejo to do much about this, I don't think, but I figured it's at least worth noting. I think because the forgejo API doesn't use `required` to indicate which response keywords we can rely upon to exist, pyforgejo winds up using `typing.Optional` for every property of every response object. This means that, technically, you can't rely on *anything* not being `None`, and type checkers will complain if you do, like this: ``` src/ai_code_review/core/forgejo_client.py:109: error: Argument "id" to "PullRequestInfo" has incompatible type "int | None"; expected "int" [arg-type] src/ai_code_review/core/forgejo_client.py:110: error: Argument "number" to "PullRequestInfo" has incompatible type "int | None"; expected "int" [arg-type] src/ai_code_review/core/forgejo_client.py:111: error: Argument "title" to "PullRequestInfo" has incompatible type "str | None"; expected "str" [arg-type] src/ai_code_review/core/forgejo_client.py:113: error: Item "None" of "PrBranchInfo | None" has no attribute "ref" [union-attr] src/ai_code_review/core/forgejo_client.py:113: error: Argument "source_branch" to "PullRequestInfo" has incompatible type "str | Any | None"; expected "str" [arg-type] src/ai_code_review/core/forgejo_client.py:114: error: Item "None" of "PrBranchInfo | None" has no attribute "ref" [union-attr] src/ai_code_review/core/forgejo_client.py:114: error: Argument "target_branch" to "PullRequestInfo" has incompatible type "str | Any | None"; expected "str" [arg-type] src/ai_code_review/core/forgejo_client.py:115: error: Item "None" of "User | None" has no attribute "login" [union-attr] src/ai_code_review/core/forgejo_client.py:115: error: Argument "author" to "PullRequestInfo" has incompatible type "str | Any | None"; expected "str" [arg-type] src/ai_code_review/core/forgejo_client.py:116: error: Argument "state" to "PullRequestInfo" has incompatible type "str | None"; expected "str" [arg-type] src/ai_code_review/core/forgejo_client.py:117: error: Argument "web_url" to "PullRequestInfo" has incompatible type "str | None"; expected "str" [arg-type] ``` these are all because the types in pyforgejo's `PullRequest` model (where I'm getting the values from) are `typing.Optional[int]`, `typing.Optional[str]` etc. Making *all* your code (somewhat nonsensically) defend against things like the pull request number potentially being `None` is a bit of a drag. Specifically I think this happens because there's no real way to indicate in a Python class that it *may* or *may not* have a given property with particular attributes, so instead fern gives properties that aren't `required` in the API definition a default value of `None` and makes them `typing.Optional`, but I'm guessing a bit there, fern is a huge codebase and it's hard to find the bit that does this. It's also worth noting that forgejo's API definition does not indicate which response properties are nullable. Technically it's saying that *none* of them are nullable; a literal reading of the API definition is something like "you can't rely on any of these properties being in the response at all, but if they are, none of them will be null". Which is obviously not true. The real situation seems to be more like "you can always rely on all the properties being in the response, but some of them might be null".
Sign in to join this conversation.
No Branch/Tag specified
main
1.0
2.0.7
2.0.6
2.0.5
2.0.4
2.0.3
2.0.2
2.0.1
2.0.0
1.0.4
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
harabat/pyforgejo#42
Reference in a new issue
harabat/pyforgejo
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?