Skip to content

Fix pyright errors in arcade.gl - #1767

Merged
einarf merged 6 commits into
pythonarcade:developmentfrom
cspotcode:fix-pyright-errors-arcade-gl
May 18, 2023
Merged

Fix pyright errors in arcade.gl#1767
einarf merged 6 commits into
pythonarcade:developmentfrom
cspotcode:fix-pyright-errors-arcade-gl

Conversation

@cspotcode

Copy link
Copy Markdown
Collaborator

Followup to #1751. Will need a rebase after #1751 gets squashed. Requesting a review from @einarf , but no rush, I know you're slowly getting back into things.

@cspotcode cspotcode mentioned this pull request May 10, 2023
Comment thread arcade/cache/texture.py Outdated
Comment thread arcade/gl/context.py Outdated
Comment thread arcade/gl/program.py Outdated
Comment thread arcade/gl/types.py Outdated
Comment thread arcade/gui/examples/scroll_area.py
Comment thread arcade/gui/property.py
Comment thread arcade/gui/property.py Outdated
Comment thread arcade/gui/widgets/layout.py
Comment thread arcade/gui/widgets/slider.py Outdated
Comment thread arcade/texture/transforms.py Outdated
@cspotcode

Copy link
Copy Markdown
Collaborator Author

@gran4 Thanks for taking a look, but unfortunately, this PR can't be reviewed yet. It's stacked on top of #1751 so the diff is all wonky. It'll be reviewable once #1751 merges.

@gran4

gran4 commented May 11, 2023

Copy link
Copy Markdown
Contributor

Oh, ok you should make it a draft then. (unless there is something I don't understand)

@einarf

einarf commented May 11, 2023

Copy link
Copy Markdown
Member

The gl stuff looks perfectly reasonable 👍

@cspotcode
cspotcode marked this pull request as draft May 11, 2023 18:19
@cspotcode

Copy link
Copy Markdown
Collaborator Author

@einarf Thanks for taking a look! I marked this PR as draft while we wait for #1751 to merge. Once #1751 is merged, I'll rebase this PR, bring it out of draft, and ping you for a merge.

@cspotcode
cspotcode force-pushed the fix-pyright-errors-arcade-gl branch from d48147a to 8a6d8a3 Compare May 18, 2023 01:10
@cspotcode

Copy link
Copy Markdown
Collaborator Author

I rebased yesterday, but there are still one or two mypy errors I need to understand and fix before this can merge. I'll solicit a review after I've fixed them.

@cspotcode

cspotcode commented May 18, 2023

Copy link
Copy Markdown
Collaborator Author

@einarf I need some help understanding some of the gl types.

f"Program attribute '{prog_attr.name}' has type {gl_name(prog_attr.gl_type)} "
f"while the buffer description has type {gl_name(attr_descr.gl_type)}. "

I get a typecheck diagnostic Argument 1 to "gl_name" has incompatible type "Optional[int]"; expected "int"
This is because prog_attr.gl_type is Optional[int]

Is that correct, should AttribFormat.gl_type be Optional?

AttribFormat is instantiated here:

arcade/arcade/gl/program.py

Lines 394 to 401 in 889772d

AttribFormat(
c_name.value.decode(),
type_info.gl_type,
type_info.components,
type_info.gl_size,
location=location,
)
)

and here:

arcade/arcade/gl/types.py

Lines 311 to 314 in 889772d

AttribFormat(
attr_name, gl_type, components, byte_size, offset=self.stride
)
)

For the latter, gl_type is possibly None because it comes from _formats, which has some None values in x1, x2, x4, x8

arcade/arcade/gl/types.py

Lines 196 to 220 in 889772d

_formats: dict[str, tuple[Optional[PyGLenum], int]] = {
# (gl enum, byte size)
# Floats
"f": (gl.GL_FLOAT, 4),
"f1": (gl.GL_UNSIGNED_BYTE, 1),
"f2": (gl.GL_HALF_FLOAT, 2),
"f4": (gl.GL_FLOAT, 4),
"f8": (gl.GL_DOUBLE, 8),
# Unsigned integers
"u": (gl.GL_FLOAT, 4),
"u1": (gl.GL_FLOAT, 1),
"u2": (gl.GL_FLOAT, 2),
"u4": (gl.GL_FLOAT, 4),
# Signed integers
"i": (gl.GL_INT, 4),
"i1": (gl.GL_BYTE, 1),
"i2": (gl.GL_SHORT, 2),
"i4": (gl.GL_INT, 4),
# Padding (1, 2, 4, 8 bytes)
"x1": (None, 1),
"x2": (None, 2),
"x4": (None, 4),
"x8": (None, 8),
}

@einarf
einarf marked this pull request as ready for review May 18, 2023 16:44
@einarf
einarf merged commit dfe8cfc into pythonarcade:development May 18, 2023
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.

3 participants