chmarti1 / PYroMat

PYroMat thermodynamic properties in Python
http://pyromat.org
Other
71 stars 13 forks source link

Discussion: Error Codes #80

Open jranalli opened 1 year ago

jranalli commented 1 year ago

As we're trying to write more front-end friendly code (e.g. PMGI), I'm finding that the error cases we have don't quite provide enough info. PMParamError can mean you specified P, T and d. It can also mean you specified an insane property called JX.

Proposed solutions: more error types or embed concrete error codes with the error so client code can avoid trying to process strings.

Happy to dig into either solution pending discussion.

chmarti1 commented 1 year ago

I think the better approach is to provide mechanisms for the error messages to be passed to the front end. Seeing the error type is helpful, but a meaningful message is always better. The other advantage is that avoids needing to dig through ALL of the code to rewrite all error handling to be consistent with a new type.

jranalli commented 1 year ago

I don't think it would be possible to get the error messages to do all the lifting on this, because the users have different goals in mind. As an example, when too many parameters are specified, I'd like our educational front end code to be very elaborate in the explanation and maybe even link to a page about the state principle, while a CLI user would probably not like that maximum verbosity error message.

We could say "just parse the string to figure out which type it was", but that would require that the exact text of error messages is guaranteed immutable in future versions. There are also different exact forms of the text that are used by different PM classes and different parts of the code within those, so we'd need to clean them up.

This is what I really had in mind, because I believe it's simpler than trying to guarantee parsable messages:

class PMParamError(Error):
    def __init__(self, message, subcode=0):
        super().__init__(message)
        self.subcode = subcode

We could then agree on just a few codes for a few specific cases (e.g. state principle violations as compared to temperature-in-bounds violations). And any client code that cares can now just check for error.code, and any code that doesn't care doesn't need to change. Specifying a default/type_unspecified code would prevent us from having to change every single instance of PMParamError, but would still allow new specific codes to be added piecemeal if any other cases of these come up. And to be clear, I am volunteering to be the one who goes through the relevant parts of the codebase to add these codes, so it's not necessarily a big time suck for you. I think these are all going to come from the various substance class files.

A quick look through mp1.py and ig.py indicates that it could be as simple as 6 total cases for PMParamError to cover those whole files:

Sidebar: the other way to do it piecemeal without breaking things would be if you wanted to actually make these new error types that inherit from PMParamError. Any code that tries to catch PMParamError would still catch those subtypes, but that seems ickier and more prone to unexpected results to me.