nasa / fprime-tools

F´ Python tooling and helpers.
https://github.com/nasa/fprime
Apache License 2.0
21 stars 39 forks source link

Add `flake8-error-message` convention #138

Closed ThibFrgsGmz closed 1 year ago

ThibFrgsGmz commented 1 year ago
Originating Project/Creator
Affected Component
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n)
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

This feature implements the flake8-err-msg which allows the formatting of nice error messages.

Rationale

The problem is that Python includes the line with the lift in the default traceback. This means that a user receives a message like this:

sub = "Some value"
raise RuntimeError(f"{sub!r} is incorrect")
Traceback (most recent call last):
  File "tmp.py", line 2, in <module>
    raise RuntimeError(f"{sub!r} is incorrect")
RuntimeError: 'Some value' is incorrect

If it is longer or more complex, the duplication can be confusing for a user not used to reading tracebacks.

On the other hand, if you always assign to something like msg, you get :

sub = "Some value"
msg = f"{sub!r} is incorrect"
raise RunetimeError(msg)
Traceback (most recent call last):
  File "tmp.py", line 3, in <module>
    raise RuntimeError(msg)
RuntimeError: 'Some value' is incorrect

There is now a more straightforward trace, less code and no double messages.

Ref - Ruff linter

Testing/Review Recommendations

Inspection is enough

Future Work

https://github.com/fprime-community/fprime-tools/pull/140

thomas-bc commented 1 year ago

See discussion in https://github.com/fprime-community/fprime-gds/pull/129

LeStarch commented 1 year ago

@thomas-bc merge when you are ready!

LeStarch commented 1 year ago

We agreed that these changes are good, however, as a note: in F´  we expect to trap exceptions and present a clean error message to users without a confusing stack-trace. Thus the motivation here becomes one of appeasing a potentially good linter and less to beautify messages for users directly.