Skip to content

Improved error handling, multiprocess environment setup, and pytest fixes#63

Merged
sophiemiddleton merged 13 commits intoMu2e:mainfrom
sam-grant:sgrant/error-handling
Feb 19, 2026
Merged

Improved error handling, multiprocess environment setup, and pytest fixes#63
sophiemiddleton merged 13 commits intoMu2e:mainfrom
sam-grant:sgrant/error-handling

Conversation

@sam-grant
Copy link
Collaborator

Improved error handling, multiprocess environment setup, and pytest fixes

Error handling (pyprocess.py)

  • Skip failed files in parallel processing instead of crashing the batch
  • Add warnings for None worker returns
  • Fix Skeleton.execute() falsely logging success when results are None
  • Cap max_workers to file list length

Environment setup (_env_manager.py)

  • Replace Python-global ENV_IS_SETUP with os.environ["PYUTILS_ENV_SETUP"]: spawned worker processes now inherit setup state from the parent and skip redundant re-initialisation

Bug fixes (pyread.py, pyimport.py)

  • Correct log level for file open failures
  • Fix UnboundLocalError in finally block when file open fails before file variable is assigned
  • Various minor fixes to test module (pytest.py)

Copy link

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 pull request improves error handling and multiprocess environment setup in the pyutils data processing framework, along with several bug fixes in file handling and test suite improvements.

Changes:

  • Replaced Python-global ENV_IS_SETUP variable with os.environ["PYUTILS_ENV_SETUP"] to properly propagate environment setup state to spawned worker processes
  • Modified parallel file processing to skip failed files and continue processing the batch rather than crashing on first failure
  • Fixed UnboundLocalError in pyimport.py and corrected log level in pyread.py for file open failures
  • Cleaned up test suite by removing duplicate method definition, adding test for corrupted files, and adjusting test configurations

Reviewed changes

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

Show a summary per file
File Description
pyutils/_env_manager.py Replaced module-level global ENV_IS_SETUP with os.environ["PYUTILS_ENV_SETUP"] to ensure spawned processes inherit environment setup state
pyutils/pyprocess.py Enhanced error handling: skip failed files in parallel processing, add warnings for None returns, cap max_workers to file list length, fix Skeleton.execute() success logging
pyutils/pyread.py Changed log level from "warning" to "error" for file open exceptions since they're immediately re-raised
pyutils/pyimport.py Fixed UnboundLocalError by initializing file = None before try block so finally block can safely reference it
tests/pytest.py Removed duplicate _remote_process_file method, added test for corrupted files with expect_return=False, enabled emoji logging, fixed file paths and test configurations
tests/run.ipynb Updated test execution output reflecting new test data files and test results (output only, no code changes)

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

Copy link
Collaborator

@sophiemiddleton sophiemiddleton left a comment

Choose a reason for hiding this comment

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

Looks good. Do you have time on mayne friday to discuss tests and examples?

@sam-grant
Copy link
Collaborator Author

Looks good. Do you have time on mayne friday to discuss tests and examples?

Yes, I can do that. I'm busy throughout the morning, but have time in the afternoon.

@sophiemiddleton sophiemiddleton merged commit bc4902d into Mu2e:main Feb 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants