Skip to content

Conversation

Flamefire
Copy link
Contributor

This is especially useful for Pytorch tests where the error message contains formatted output which becomes hard to read:

== FAILED: Installation ended unsuccessfully: An error was raised during test step: 'Failing because not all failed tests could be determined. Tests failed to start, crashed or the test accounting in the PyTorch EasyBlock needs updating!\nMissing: distributed/test_c10d_nccl\nYou can check the test failures (in the log) manually and if they are harmless, use --ignore-test-failure to make the test step pass.\n109 test failures, 0 test errors (out of 262294):\nFailed tests (suites/files):\n\tdistributed/test_symmetric_memory (23 failed, 20 passed, 1 skipped, 0 errors)\n\tdynamo/test_misc (1 failed, 539 passed, 12 skipped, 0 errors)\n\tdynamo/test_utils (1 failed, 7 passed, 0 skipped, 0 errors)\n\tinductor/test_codecache (1 failed, 110 passed, 3 skipped, 0 errors)\n\tinductor/test_cudacodecache (3 failed, 0 passed, 0 skipped, 0 errors)\n\tinductor/test_cutlass_backend (39 failed, 1 passed, 2 skipped, 0 errors)\n\tinductor/test_fp8 (40 failed, 149 passed, 72 skipped, 0 errors)\n\tinductor/test_torchinductor_dynamic_shapes (1 failed, 1527 passed, 71 skipped, 0 errors)\nCould not count failed tests for the following test suites/files:\n\tdistributed/test_c10d_nccl (Undetected or did not run properly)' (took 64 hours 17 mins 9 secs)

Some exception texts have formatting such as quotes, newlines, tabs.
Those should be preserved when reporting the error.
This ensures that formatting is kept
This ensures any formatting like newlines is kept.
@Flamefire Flamefire force-pushed the improve-error-reporting branch from d9dd161 to 8be2c5d Compare September 29, 2025 08:33

def __str__(self):
"""Return string representation of this EasyBuildError instance."""
return repr(self.msg)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, there must have been a reason why we did this...

Copy link
Contributor Author

@Flamefire Flamefire Oct 8, 2025

Choose a reason for hiding this comment

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

C&P from some other code I'd say as this was introduced by renaming some other exception class in the very beginning: https://github.com/easybuilders/easybuild-framework/blame/d7f1ad2f6f09e387e121c94c9df434a07e7aaa68/easybuild/tools/buildLog.py

There is a __repr__ special method that should likely be implemented like that.

Someone notes at https://stackoverflow.com/questions/1436703/what-is-the-difference-between-str-and-repr

default for __repr__ which would act like:

return "%s(%r)" % (self.__class__, self.__dict__)

__repr__ goal is to be unambiguous
__str__ goal is to be readable
This means, in simple terms: almost every object you implement should have a functional __repr__ that’s usable for understanding the object. Implementing __str__ is optional: do that if you need a “pretty print” functionality (for example, used by a report generator).
Summary

Implement __repr__ for any class you implement. This should be second nature. Implement __str__ if you think it would be useful to have a string version which errs on the side of readability.

E.g.

def __repr__(self):
    return f'EasyBuildError({self.msg!r}, {self.exit_code})'

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants