Skip to content

Improve errors #72

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

Merged
merged 5 commits into from
Aug 18, 2024
Merged

Improve errors #72

merged 5 commits into from
Aug 18, 2024

Conversation

smartycope
Copy link
Contributor

As I was developing a program using pyndustric, I kept getting errors which were hard to read or with insufficient information. This update adds some information, makes error messages easier to read, and allows for custom descriptions to CompilerErrors.

Copy link
Owner

@Lonami Lonami left a comment

Choose a reason for hiding this comment

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

I'm not to keen on the changes to the numeric error codes. Now we have an internal constant name, a user-facing constant, and a description, all of which differ.

My idea with numeric codes was, they'd be consistent and easy to reference. I'm curious why the description was not enough?

Co-authored-by: Lonami <[email protected]>
@smartycope
Copy link
Contributor Author

I figured it would be more Pythonic. Since the error codes (E003, etc.) are arbitrary, why not make them arbitrary in a way that is a little more useful? In addition, there's several instances where more description of the error would be useful, as opposed to a standard error description.

@Lonami
Copy link
Owner

Lonami commented Aug 17, 2024

I figured it would be more Pythonic. Since the error codes (E003, etc.) are arbitrary

Only partially arbitrary. The prefix indicates it's an error, and the rest is an increasing unique ID. I don't remember my rationale, but I was probably inspired by Rust (see https://doc.rust-lang.org/error_codes/error-index.html and https://rustwiki.org/en/error-index).

Seems stuff like ruffs has both codes and friendly names https://docs.astral.sh/ruff/rules. But I'm not sure how much that matters for us, since none of the errors can be disabled or turned into warnings.

there's several instances where more description of the error would be useful

No question here, but providing useful feedback isn't easy, so I haven't really prioritised that. I figured the code and position where the error occured might've been enough information. Along with a generic explanation of the problem of course.

Also, I made no promises of the error codes being stable, and I doubt anyone would care if we changed them right now, but it still feels wrong to straight up replace all of them. I don't know. WDYT?

@smartycope
Copy link
Contributor Author

You bring up a good point. Tools like black, flake8, and compilers, and other formatters tend to identify errors by arbitrary codes, but I figured they do because it makes it easier to specify specific errors via the command line. We're not doing that at all though.

I wouldn't think replacing them would be a big deal, because they're only used internally. Nothing really changes from the user's perspective.

@Lonami Lonami merged commit cf3095c into Lonami:master Aug 18, 2024
2 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.

2 participants