Skip to content

Fix module_exists on Python 3.12#179

Open
pilipilisbot wants to merge 1 commit into
masterfrom
175/fix-python312-imp
Open

Fix module_exists on Python 3.12#179
pilipilisbot wants to merge 1 commit into
masterfrom
175/fix-python312-imp

Conversation

@pilipilisbot
Copy link
Copy Markdown
Contributor

Objectiu

Corregir la compatibilitat amb Python 3.12, on el mòdul estàndard imp ja no existeix.

Refs #175

Canvis

  • module_exists() usa importlib.util.find_spec() quan està disponible.
  • Es manté fallback amb imp.find_module() per Python 2.7.
  • La neteja de netsvc.SERVICES queda protegida perquè els tests unitaris puguin executar-se sense entorn ERP carregat.
  • S'afegeixen tests unitaris per mòdul existent, submòdul existent i mòdul inexistent.
  • S'elimina un SyntaxWarning de Python 3.12 en el patró regex de find_files().

Validació

Executat correctament:

# Python 2.7.18
python -m unittest discover tests

# Python 3.11.15
python -m unittest discover tests

# Python 3.12.3
python3 -m unittest discover tests

git diff --check

No he executat mamba spec perquè mamba no està instal·lat en aquest entorn local.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates destral.utils to be compatible with Python 3.12 by removing the hard dependency on the removed stdlib module imp, while keeping a Python 2.7 fallback, and adds regression tests around the updated behavior.

Changes:

  • Update module_exists() to use importlib.util.find_spec() when available (and keep an imp.find_module() fallback for Python 2.7).
  • Guard netsvc.SERVICES cleanup so unit tests can run without a loaded ERP environment.
  • Add unit tests for module_exists() and fix a Python 3.12 SyntaxWarning in find_files()’ regex literal.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
destral/utils.py Switch module_exists() to importlib.util.find_spec(), add guarded netsvc cleanup, and adjust find_files() regex literal.
tests/test_utils.py Add unit tests covering module_exists() for existing modules/submodules and missing modules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread destral/utils.py Outdated
Comment thread destral/utils.py Outdated
Use importlib.util.find_spec on Python 3 to avoid the removed imp module, keeping the imp fallback for Python 2.7.

Add regression coverage for existing and missing modules.

Co-authored-by: Eduard Carreras <ecarreras@gisce.net>
@pilipilisbot pilipilisbot force-pushed the 175/fix-python312-imp branch from f03d109 to ac7eb34 Compare May 3, 2026 20:58
@pilipilisbot
Copy link
Copy Markdown
Contributor Author

He aplicat els dos comentaris de Copilot i he fet force-push del commit actualitzat ac7eb34:

  • find_files() torna a usar patró unicode amb els + escapats per evitar el problema Python 2 amb diff unicode.
  • Afegit test específic amb diff unicode.
  • Corregida la gramàtica del docstring de module_exists().
  • He resolt els dos review threads.

Validació local repetida:

  • Python 2.7.18: python -m unittest discover tests OK
  • Python 3.11.15: python -m unittest discover tests OK
  • Python 3.12.3: python3 -m unittest discover tests OK
  • git diff --check OK

CI GitHub del commit nou també OK: tests-py27 i tests-py311 verds.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug patch Patch auto version V_._.X py3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants