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

style: Fix if-else-block-instead-of-if-exp (SIM108) (part 2) #4562

Merged

Conversation

echoix
Copy link
Member

@echoix echoix commented Oct 20, 2024

Part 2 of #4561

This PR fixes SIM108 for the scripts/* and python/* folders

Ruff rule: https://docs.astral.sh/ruff/rules/if-else-block-instead-of-if-exp

@echoix echoix requested a review from ninsbl October 20, 2024 21:21
@github-actions github-actions bot added vector Related to vector data processing raster Related to raster data processing Python Related code is in Python database Related to database management libraries module general display imagery tests Related to Test Suite raster3d notebook misc labels Oct 20, 2024
Copy link
Member

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of lines with:

id = input if input.find("@") >= 0 else input + "@" + mapset

Theres are maybe better written as:

id = input if "@" in input else f"{input}@{mapset}"

But I am not sure if tthat kind of changes are out of scope, so if you rather keep that separate that is fine with me...

python/grass/temporal/univar_statistics.py Show resolved Hide resolved
python/grass/temporal/temporal_algebra.py Show resolved Hide resolved
python/grass/temporal/temporal_algebra.py Show resolved Hide resolved
python/grass/temporal/sampling.py Show resolved Hide resolved
python/grass/temporal/sampling.py Show resolved Hide resolved
@echoix
Copy link
Member Author

echoix commented Oct 21, 2024

Of course, I know about them, and the in is more performant. I saw someone on stack overflow that showed numbers with 10x better (not recent though). I didn't want to shove them in hidden in these big PR (enough to split them in 3). I hoped that the PRs of this weekend would have been merged rapidely so I would have the chance to make the find-> in changes.

So, once this PR, and the following (ready, but not posted yet) are merged, I'll do the changes for str.find() to in. There are some instances of that pattern too if I remember well.

@echoix echoix merged commit 67a1609 into OSGeo:main Oct 21, 2024
26 checks passed
@echoix echoix deleted the fix-SIM108-if-else-block-instead-of-if-exp-part2 branch October 21, 2024 21:07
@github-actions github-actions bot added this to the 8.5.0 milestone Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Related to database management display general imagery libraries misc module notebook Python Related code is in Python raster Related to raster data processing raster3d tests Related to Test Suite vector Related to vector data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants