Pylint: global-statement, no-else-raise, unneccessary-pass

[evaherrada]

Here's pylint's output:

************* Module adafruit_rplidar
adafruit_rplidar.py:108:0: R0205: Class 'RPLidar' inherits from object, can be safely removed from bases in python3 (useless-object-inheritance)
adafruit_rplidar.py:143:12: W0603: Using the global statement (global-statement)
adafruit_rplidar.py:144:12: C0415: Import outside toplevel (serial) (import-outside-toplevel)
adafruit_rplidar.py:239:8: R1720: Unnecessary "elif" after "raise" (no-else-raise)
adafruit_rplidar.py:314:8: W0107: Unnecessary pass statement (unnecessary-pass)

As far as the useless-object-inheritance goes, @brentru I should be able to do the same thing you did in adafruit/Adafruit_CircuitPython_CursorControl#17, correct?

For global-statement and import-outside-toplevel, here's the offending code:

140        if self.is_CP:
141            _serial_port = port
142        else:
143           global serial
144           import serial

If I understand correctly, this checks if the board is a circuitpython board and if it isn't, it imports serial, although I'm not sure why line 143 is necessary.

For no-else-raise:

239        if len(descriptor) != DESCRIPTOR_LEN:
240            raise RPLidarException("Descriptor length mismatch")
241        elif not descriptor.startswith(SYNC_BYTE + SYNC_BYTE2):
242            raise RPLidarException("Incorrect descriptor starting bytes")

I'm not sure why this is an issue, it makes sense to me why this would be done, although I don't understand this code well enough to make an informed decision on that.

For unneccessary-pass

314    def clear_input(self):
315       """Clears input buffer by reading all available data"""
316       pass

Unless there's some weird python mechanic I don't understand going on here, I think that this would be the right place to raise a NotImplementedError.

Referencing main issue: adafruit/Adafruit_CircuitPython_Bundle#232

[tannewt]

Here's pylint's output:

************* Module adafruit_rplidar
adafruit_rplidar.py:108:0: R0205: Class 'RPLidar' inherits from object, can be safely removed from bases in python3 (useless-object-inheritance)
adafruit_rplidar.py:143:12: W0603: Using the global statement (global-statement)
adafruit_rplidar.py:144:12: C0415: Import outside toplevel (serial) (import-outside-toplevel)
adafruit_rplidar.py:239:8: R1720: Unnecessary "elif" after "raise" (no-else-raise)
adafruit_rplidar.py:314:8: W0107: Unnecessary pass statement (unnecessary-pass)

As far as the useless-object-inheritance goes, @brentru I should be able to do the same thing you did in adafruit/Adafruit_CircuitPython_CursorControl#17, correct?

Yes, you should be able to remove the (object) from the class line. It's a python2ism that got here somehow.

For global-statement and import-outside-toplevel, here's the offending code:

140        if self.is_CP:
141            _serial_port = port
142        else:
143           global serial
144           import serial

If I understand correctly, this checks if the board is a circuitpython board and if it isn't, it imports serial, although I'm not sure why line 143 is necessary.

I don't believe the global piece is needed either because the if doesn't create a new scope. I'd disable those pylint checks by adding a comment at the end of the relevant line, which should limit that disable to the single line.

For no-else-raise:

239        if len(descriptor) != DESCRIPTOR_LEN:
240            raise RPLidarException("Descriptor length mismatch")
241        elif not descriptor.startswith(SYNC_BYTE + SYNC_BYTE2):
242            raise RPLidarException("Incorrect descriptor starting bytes")

I'm not sure why this is an issue, it makes sense to me why this would be done, although I don't understand this code well enough to make an informed decision on that.

This seems similar to the unnecessary else case when returning from the previous if because with return and raise the function is finished. Here change the elif to an if and it should stop complaining and be equivalent.

Here is the similar return case:

if foo:
  return True
else:
  return bar

Should be:

if foo:
  return True
return bar

For unneccessary-pass

314    def clear_input(self):
315       """Clears input buffer by reading all available data"""
316       pass

Unless there's some weird python mechanic I don't understand going on here, I think that this would be the right place to raise a NotImplementedError.

Referencing main issue: adafruit/Adafruit_CircuitPython_Bundle#232

Returning NotImplementedError would be ok but isn't related to the lint error. For things that define a new block (if,def,for,while, etc) Python requires the next line to be indented since blocks are defined by indentation. pass is a way to have that required line but do nothing. In this case, the comment already counts as the single line of indentation so the pass isn't needed and lint is erroring on it. The simplest fix is to remove the pass. I'd add the raise NotImplementedError if the expectation is that a subclass override the function. Otherwise, an empty function or one without returns will return None automatically.

[evaherrada]

@tannewt Thanks! That was actually quite informative. I'll make the changes you've suggested.

[kattni]

This is completed.