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: Fix E722 do not use bare 'except' warnings in gui_core/ #4396

Merged
merged 14 commits into from
Oct 1, 2024

Conversation

arohanajit
Copy link
Contributor

Description:

This PR addresses the E722 do not use bare 'except' warnings by specifying the exception types to be caught in the following files in gui_core/ directory:

  1. dialogs.py:

    • Change: Specified re.error for regular expression operations.
  2. forms.py:

    • Change: Specified Exception as a general catch-all.
    • Rationale: Using a general exception handler as there could be multiple applicable exceptions (two I could see were IndexError,AttributeError)
  3. ghelp.py:

    • Change: Specified KeyError for dictionary operations, a general catchall Exception for everything else
  4. gselect.py:

    • Change: Specified Exception as a general catch-all.
    • Rationale: Using a general exception handler as there could be multiple applicable exceptions (potential errors from wx module)
  5. widgets.py:

    • Change: Specified ValueError for datetime parsing.

Rationale:

  • Specific Exception Handling: Improves code clarity and robustness by handling only relevant exceptions.
  • Code Quality: Adheres to PEP 8 guidelines, ensuring better maintainability and readability.

Please review the changes and let me know if any further modifications are needed.

@github-actions github-actions bot added GUI wxGUI related Python Related code is in Python labels Sep 27, 2024
@echoix
Copy link
Member

echoix commented Sep 30, 2024

Do you happen to know if there's an extra flake8 exclusion removed here?

@echoix
Copy link
Member

echoix commented Sep 30, 2024

Oh, I see that another wxGUI PR that I enabled automerge on would touch that line, so it's sure a conflict will happen. So probably this and some other PRs will need to have their conflicts fixed once that other PR is merged. But that other PR is queued after this and other ones.

@arohanajit
Copy link
Contributor Author

Sure, I will go through the PRs and fix the merge conflicts when and where they happen

petrasovaa
petrasovaa previously approved these changes Sep 30, 2024
@echoix
Copy link
Member

echoix commented Sep 30, 2024

Conflicts to solve.

@echoix echoix enabled auto-merge (squash) October 1, 2024 03:15
@echoix echoix merged commit 25619dd into OSGeo:main Oct 1, 2024
26 checks passed
@arohanajit arohanajit deleted the gui_core_722 branch October 1, 2024 15:48
@petrasovaa petrasovaa added this to the 8.5.0 milestone Oct 1, 2024
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