bme280 icon indicating copy to clipboard operation
bme280 copied to clipboard

Adding Typing // Dataclasses // Python 3.5+ compat

Open miili opened this issue 3 years ago • 3 comments

Hi there,

thanks for providing this library.

  • I added dataclass for the calibration parameters and removed the obscure attrib dictionary. This is more explicit and safer to use (and avoids a memory leak in dict subclassing). https://peps.python.org/pep-0557/
  • I replaced the oversampling for simpler IntEnum from std library. https://docs.python.org/3/library/enum.html
  • I removed the pytz dependency in favor of standard datetime.timezone. This is a nightmare in python but datetime.now(timezone.utc) does what is needed here. Now there are no other dependencies than smbus2 :+1: . https://docs.python.org/3/library/datetime.html#datetime.datetime.utcnow
  • I added native typing from __future__ which makes the code safer to use. https://peps.python.org/pep-0585/
  • I used black to format the code according to PEP8. https://black.readthedocs.io/
  • I changed class naming to CamelCase and prefixed internal functions with single _. Double __ is for python internals.

Please comment on this MR or request further changes.

Thanks again for this lib, it saved us a lot of time :) I hope this PR will improve the usability of the library

miili avatar Mar 11 '22 15:03 miili

In principle, yes looks good :+1:, but I guess my first thought is that will it cause breaking changes? .. if so, is it possible to put a compatibility layer so that folks can still use it unmodified? (I mean this is probably not a huge issue, as we could bump the major version and put a banner up indicating what changed)

rm-hull avatar Mar 11 '22 17:03 rm-hull

Tests failed though https://github.com/rm-hull/bme280/runs/5514636316?check_suite_focus=true

rm-hull avatar Mar 11 '22 17:03 rm-hull

Yes __future__.annotations is only available from 3.7.

And yes, surely the minor version has to be bump, if not the major. Any production system downstream should fix the dependency version of this library anyways.

miili avatar Mar 11 '22 19:03 miili