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

wxGUI: Fixed E722 in frame.py #4440

Merged
merged 7 commits into from
Oct 10, 2024
Merged

wxGUI: Fixed E722 in frame.py #4440

merged 7 commits into from
Oct 10, 2024

Conversation

arohanajit
Copy link
Contributor

Fixed E722 by specifying exceptions

  • AttributeError incase object does not have .name or GetLayerInfo() attribute
  • KeyError incase object does not have mapplayer attribute
  • ValueError and IndexError for invalid dim types and indexing

@github-actions github-actions bot added GUI wxGUI related Python Related code is in Python labels Oct 3, 2024
@arohanajit arohanajit changed the title wxGUi: Fixed E722 in frame.py wxGUI: Fixed E722 in frame.py Oct 3, 2024
@echoix
Copy link
Member

echoix commented Oct 4, 2024

Wait until we merge #4439 before updating this one, and at that time you could adjust the flake8 file for exclusions. If you do it before, the CI will be useless as there will be conflicts on that file.

@petrasovaa petrasovaa added this to the 8.5.0 milestone Oct 4, 2024
@echoix
Copy link
Member

echoix commented Oct 4, 2024

Ok, finally #4439 ended up finishing and merging, so time to update this one and its flake8

@echoix
Copy link
Member

echoix commented Oct 9, 2024

Is it expected that no flake8 exclusions were removed from the config file here?

@arohanajit
Copy link
Contributor Author

arohanajit commented Oct 9, 2024

I noticed that a lot of merge conflicts were happening since different PRs were being merged with different modifications to .flake8. Since I'll be working with this file for a while, I thought I'd try removing these entries from .flake8 post merge once I move on to next set of errors. Please let me know if you'd rather this be the way it was before

@echoix
Copy link
Member

echoix commented Oct 9, 2024

What I usually do when having this kind of conflicts happening with ruff exclusions, is to do a PR, skip a few lines, do a PR for the other, skip a few lines, make a PR, and wait for others to be merged before filing them in. Sometimes I have them already made, but waiting in my fork before sending them.

Or I find a second front to work on in the meantime that doesn't touch the same files.

@petrasovaa petrasovaa merged commit cbe2b9e into OSGeo:main Oct 10, 2024
26 checks passed
@arohanajit arohanajit deleted the 722-frame branch October 11, 2024 05:01
@petrasovaa
Copy link
Contributor

Hm, looks like we missed TypeError:

Traceback (most recent call last):
  File "/home/akratoc/dev/grass/grass_main/dist.x86_64-pc-
linux-gnu/gui/wxpython/lmgr/layertree.py", line 1759, in
OnDeleteLayer

if self.GetLayerInfo(item, key="type") != "group":
  File "/home/akratoc/dev/grass/grass_main/dist.x86_64-pc-
linux-gnu/gui/wxpython/lmgr/layertree.py", line 337, in
GetLayerInfo

return self.GetPyData(layer)[0][key]

TypeError
:
'NoneType' object is not subscriptable

@arohanajit would you mind creating a PR that adds TypeError in cases GetLayerInfo is used in try/except blocks?

@arohanajit
Copy link
Contributor Author

Yes, I'll push one PR with this change today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI wxGUI related Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants