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

python: More fixes for Python 3.12 SyntaxWarning: invalid escape sequence #3331

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

echoix
Copy link
Member

@echoix echoix commented Jan 4, 2024

Continuing on the same trend as point 13 of #3316, this solves other SyntaxWarnings of escapes sequences. I noticed them in the output of make, locally.

It is probably not all of them yet, but warnings are not flagged for each instance of errors, it seems one per file (and if it showed up during an import, it won't show up again). I didn't know about these escape sequences and weren't present in the warnings last time I checked.

This is a side quest, but I cherry-picked my commit to make a quick PR.

@github-actions github-actions bot added GUI wxGUI related Python Related code is in Python libraries labels Jan 4, 2024
python/grass/script/task.py Outdated Show resolved Hide resolved
@echoix
Copy link
Member Author

echoix commented Jan 5, 2024

There are still some valid test failures that seem to complain about single literal integer value
https://github.com/OSGeo/grass/actions/runs/7423884108/job/20202299742?pr=3331#step:11:213

That's weird, but related to the regex change

@nilason
Copy link
Contributor

nilason commented Jan 5, 2024

There are still some valid test failures that seem to complain about single literal integer value https://github.com/OSGeo/grass/actions/runs/7423884108/job/20202299742?pr=3331#step:11:213

That's weird, but related to the regex change

The test contains variations on ranges the regex should handle.

@nilason
Copy link
Contributor

nilason commented Jan 5, 2024

There are still some valid test failures that seem to complain about single literal integer value https://github.com/OSGeo/grass/actions/runs/7423884108/job/20202299742?pr=3331#step:11:213
That's weird, but related to the regex change

The test contains variations on ranges the regex should handle.

At least we now know what CodeQL complained about.

@echoix echoix force-pushed the py312-fix-syntax-warning branch 2 times, most recently from 3c3f25d to 2444c35 Compare January 6, 2024 02:57
@echoix
Copy link
Member Author

echoix commented Jan 6, 2024

I make an intentional new black error, to see pre-commit fix the existing black formattting error

@echoix
Copy link
Member Author

echoix commented Jan 6, 2024

pre-commit.ci run

1 similar comment
@echoix
Copy link
Member Author

echoix commented Jan 6, 2024

pre-commit.ci run

@echoix
Copy link
Member Author

echoix commented Jan 6, 2024

I think why pre-commit.ci isn't able to push is the same reason that my force-pushes were rejected (saying I need a pull request, whereas I was pushing to my "checkout this pr in codespaces" branch). Is it possible that there are some other older branch protection rules that prevent it. I disabled all the rulesets on my repo in between the tries

@wenzeslaus
Copy link
Member

There is now plenty of failure reports in pre-commit.ci:

push (skipped)
pre-commit.ci tried to autofix this pr but encountered an unknown error

I think why pre-commit.ci isn't able to push is the same reason that my force-pushes were rejected...

Hm, now I remember one issue with MegaLinter automatic fixes is that it was working (1 or 2 back) only with the main repo and it would not be able to modify branches in forks, so I was applied only to branches in the main/upstream repo (which was not a problem for me).

...(saying I need a pull request, whereas I was pushing to my "checkout this pr in codespaces" branch).

Are you saying you are having problems other than pre-committ? But all these are with your branches in fork, not this repo, no?

...Is it possible that there are some other older branch protection rules that prevent it.

I disabled all the old rules for this repo when I migrated the release branch rules to a ruleset. Branches says "You haven't protected any of your branches".

@wenzeslaus
Copy link
Member

The same happens for my PR and the "unknown error" is not really helpful (no results from online search). What about "autofix_prs: false"? You can always trigger it if you want ("pre-commit.ci run") or even enable for a PR by a comment ("pre-commit.ci autofix").

@echoix
Copy link
Member Author

echoix commented Jan 6, 2024

I disabled all the old rules for this repo when I migrated the release branch rules to a ruleset. Branches says "You haven't protected any of your branches".

Good for that.

Your reformulation makes me think back of something I already encountered.

If we look here, we see the ref that is pulled.
image

refs/pull/3331 merged as a4c1a1fe8cc818b132c99dfbbdc870ce5f1fc453

There was something weird with it, and I'm not sure that its really writeable. I'll have to dig again to know what that means.

@wenzeslaus
Copy link
Member

The GitHub Actions by default run on merge with the base branch, not the PR branch. I suppose it would be the same here.

@echoix
Copy link
Member Author

echoix commented Jan 20, 2024

Can someone else take a quick look at this, it was already ok before I rebased last week. I keep getting warnings on other work, that are already solved in this.

@neteler
Copy link
Member

neteler commented Jan 22, 2024

Can someone else take a quick look at this, it was already ok before I rebased last week. I keep getting warnings on other work, that are already solved in this.

Maybe @tmszi or @landam could take a look?

@echoix
Copy link
Member Author

echoix commented Jan 22, 2024

And I'd be glad if any of my other pending PRs could be checked too, there was more than one that were ready. ;)

@echoix echoix merged commit f302d66 into OSGeo:main Jan 30, 2024
23 checks passed
@echoix echoix deleted the py312-fix-syntax-warning branch January 30, 2024 23:43
jadenabrams100 pushed a commit to ncsu-csc472-spring2024/grass-CI-playground that referenced this pull request Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI wxGUI related libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants