adafruit / Adafruit_CircuitPython_PM25

CircuitPython library for PM2.5 sensors
MIT License
28 stars 16 forks source link

Move import to I2C only section #7

Closed dglaude closed 4 years ago

dglaude commented 4 years ago

Move the import (next pylint will have to ignore this

tannewt commented 4 years ago

Why do this? If for memory saving, I'd prefer to move this library to a package with separate modules (aka files) for i2c and uart.

dglaude commented 4 years ago

I did this because of issue #5 and I got multiple suggestions on how to do it. So I gave this a try.

I am happy with "not fixing this", I would have learn to fight against Pylint and Black when they do not aggree. I am also happy with "fixing another way", as you describe... but I have never done multi-files library.

Also there is a bit of OO Python here, with a "super class" that use the I2C or UART for implementation detail, and I don't know how that work accross separated files.

tannewt commented 4 years ago

I did this because of issue #5 and I got multiple suggestions on how to do it. So I gave this a try.

I am happy with "not fixing this", I would have learn to fight against Pylint and Black when they do not agree. I am also happy with "fixing another way", as you describe... but I have never done multi-files library.

Up to you if you want to continue. Below I've outlined the steps.

Also there is a bit of OO Python here, with a "super class" that use the I2C or UART for implementation detail, and I don't know how that work accross separated files.

  1. Make an adafruit_pm25 directory.
  2. Move adafruit_pm25.py to adafruit_pm25/__init__.py
  3. Move the i2c version into a new adafruit_pm25/i2c.py file and at the top from . import PM25 to import from __init__.py. You can then import I2CDevice here too.
  4. Repeat for UART.
  5. Update the examples to from adafruit_pm25.i2c import PM25_I2C
dglaude commented 4 years ago

I also have #6 pending. When that is fix, I can try to split I2C from UART in separate files... But I don't want to make two incompatible PR.

tannewt commented 4 years ago

Ok, I merged #6. Sorry for the delay.

kattni commented 4 years ago

I'm closing this in favor of #8.