Skip to content
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

Use locally defined exception classes instead of raising generic exception types. #8

Open
Ferroin opened this issue Oct 8, 2019 · 4 comments · May be fixed by #9
Open

Use locally defined exception classes instead of raising generic exception types. #8

Ferroin opened this issue Oct 8, 2019 · 4 comments · May be fixed by #9
Labels

Comments

@Ferroin
Copy link

@Ferroin Ferroin commented Oct 8, 2019

Prompted by: netdata/netdata#7024

This module appears to directly raise ValueError and RuntimeError under specific circumstances. While both are technically accurate, they necessitate use of overly broad except clauses when dealing with error handling for the module, which makes it difficult to verify that code using the module is actually correct and can't accidentally hide other errors. The RuntimeError usage is particularly problematic, as it's very rare in the core language to ever catch a RuntimeError, so any code that catches one tends to look rather out of place to many people.

I would suggest switching to the same methodology that's used by much of the Python ecosystem of defining local sub-classes of Exception (or whatever specific error makes sense) and raising those instead of directly raising built-in exception types.

@ladyada

This comment has been minimized.

Copy link
Member

@ladyada ladyada commented Oct 8, 2019

hiya, if you'd like to submit a PR for an update, we'd love to have one - note that this code must be able to run on CircuitPython boards that have very limited RAM

@ladyada ladyada added the enhancement label Oct 8, 2019
@tannewt

This comment has been minimized.

Copy link

@tannewt tannewt commented Oct 8, 2019

@Ferroin That would be awesome to add! Let me know if you have any questions on how to do it or test it.

@Ferroin

This comment has been minimized.

Copy link
Author

@Ferroin Ferroin commented Oct 8, 2019

@tannewt Testing should be the only issue (I'm a bit out of practice with Python in general, but that shouldn't be a problem for something like this). I've never worked with CircuitPython myself (as mentioned in the issue description, this was prompted by a PR for Netdata to add a data collector that uses this module), so I'm not sure where exactly the best place to begin would be.

@tannewt

This comment has been minimized.

Copy link

@tannewt tannewt commented Oct 8, 2019

Here is a guide on how to use the sensor: https://learn.adafruit.com/adafruit-am2320-temperature-humidity-i2c-sensor/python-circuitpython

Adding the custom exceptions should be the same as CPython so it's ok to only test in CPython with Blinka.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.