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

grass.pygrass: remove unused arg in ctypes.CFUNCTYPE #4113

Merged
merged 3 commits into from
Aug 2, 2024

Conversation

mshukuno
Copy link
Contributor

Please see the comment here.

Initially, the PR was about fixing "ambiguous variable name 'l', but in this case 'l's were also unused so deletion was the initial fix. However, it appears Windows has a "ctypes" issue.

I would like to revert them as they were, but rename "l"s to avoid the E741 (Checks for the use of the characters 'l', 'O', or 'I' as variable names) issue. Since I couldn't figure out what "l" parameters are so I renamed it "l_what" for now. Let me know if you find appropriate variable name. Here is pygrass.raster package documentation.

However, the flake8 problem in F841 (unused-variable) will be addressed next, and noqa:F841 will be applied to them when I work on F841 problem.

I propose raising a separate issue if this needs to be investigated further since I don't have experience with C and "ctypes".

What do you think?

@github-actions github-actions bot added Python Related code is in Python libraries labels Jul 29, 2024
@nilason
Copy link
Contributor

nilason commented Jul 30, 2024

The reason for this to fail on the win runner only, could be the fact that that is the only one with Python 3.12.

@mshukuno mshukuno changed the title Regression PR #3928: Reverting to the original since the Windows build failed Checks: Regression PR #3928: Reverting to the original since the Windows build failed Aug 1, 2024
@nilason nilason marked this pull request as ready for review August 2, 2024 06:33
@nilason nilason enabled auto-merge (squash) August 2, 2024 06:53
@nilason nilason changed the title Checks: Regression PR #3928: Reverting to the original since the Windows build failed grass.pygrass: remove unused arg in ctypes.CFUNCTYPE Aug 2, 2024
@nilason
Copy link
Contributor

nilason commented Aug 2, 2024

@echoix I have enabled auto-merge, if you agree with me this is good to go, just approve.

@echoix
Copy link
Member

echoix commented Aug 2, 2024

I would only need to search the logs, this one, and just before, to see it fixed. I can't search on my phone, only scroll. So I'm checking soon ;) (unless someone finds the lines before).

@echoix
Copy link
Member

echoix commented Aug 2, 2024

@nilason nilason merged commit cbc8fd5 into OSGeo:main Aug 2, 2024
26 checks passed
@echoix echoix added this to the 8.5.0 milestone Aug 5, 2024
@mshukuno mshukuno deleted the regression-flake8-E741 branch August 26, 2024 14:55
Mahesh1998 pushed a commit to Mahesh1998/grass that referenced this pull request Sep 19, 2024
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants