Port changes to the number() routine in the micropython library to _number()

[hybotix]

This is a port of the changes I did to the number() routine in the micropython version of this library. I also found several errors in processing floats and negative numbers, which I fixed. This version is much more robust. I also fixed a problem with the "from adafruit_ht16k33 import..." line.

[hybotix]

PyLint does not like me.

[hybotix]

Done.

[makermelissa]

Thanks for submitting this. I've made some suggested changes that should help with it passing linting. I'm not sure why you changed it to import from matrix which imports from the base class (ht16k33) rather than just importing the ht16k33 class for better memory usage.

[makermelissa]

This is failing pylint because of all the parenthesis and variable > 0 can be shortened to variable. I'd suggest changing this line to:
if places and decimal and dot and len(stnum[places:]) > decimal:

[hybotix]

[code]if places and decimal and dot will be true as long as one of them is not zero. This is true for Python 3.7.3. I should be able to remove the parentheses without changing the meaning.

[makermelissa]

This is failing pylint because of the parenthesis and variable > 0 can be shortened to variable. I'd suggest changing this line to:
if places <= 0 and decimal:

[hybotix]

Same comment on zero checks.

[hybotix]

Hi, I have read your comments and I have a question. if a and b: print("True") will be True if a is positive and b is negative or vice versa. This would introduce a bug if for some reason one of them is negative and you *really* want to check for both > 0. This is why I explicitly check for > 0. I can drop the parentheses and everything will still come still be right. What do you think?

…

On Thu, Jan 23, 2020 at 1:59 PM Melissa LeBlanc-Williams < ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks for submitting this. I've made some suggested changes that should help with it passing linting. I'm not sure why you changed it to import from matrix which imports from the base class (ht16k33) rather than just importing the ht16k33 class for better memory usage. ------------------------------ In adafruit_ht16k33/segments.py <#53 (comment)> : > @@ -26,7 +26,8 @@ """ from time import sleep -from adafruit_ht16k33.ht16k33 import HT16K33 +#from adafruit_ht16k33.ht16k33 import HT16K33 +from adafruit_ht16k33.matrix import HT16K33 Why was this changed from the base class to the 8x8 matrix class? ------------------------------ In adafruit_ht16k33/segments.py <#53 (comment)> : > - places += 1 - self._text(string[:places]) + stnum = str(number) + dot = stnum.find('.') + + if ((len(stnum) > 5) or ((len(stnum) > 4) and (dot < 0))): + raise ValueError("Input overflow - {0} is too large for the display!".format(number)) + + if dot < 0: + # No decimal point (Integer) + places = len(stnum) + else: + places = len(stnum[:dot]) + + if self.debug: + print("(1) number = {0}, places = {1}, decimal = {2}, dot = {3}, stnum = '{4}'".format(number, places, decimal, dot, stnum)) I'd suggest removing the debug statements. They're long and make the library larger. They also seem to be the source of a lot of the linting problems. ------------------------------ In adafruit_ht16k33/segments.py <#53 (comment)> : > + ''' + Display a floating point or integer number on the Adafruit HT16K33 based displays + + Param: number - The floating point or integer number to be displayed, which must be + in the range 0 (zero) to 9999 for integers and floating point or integer numbers + and between 0.0 and 999.0 or 99.00 or 9.000 for floating point numbers. + Param: decimal - The number of decimal places for a floating point number if decimal + is greater than zero, or the input number is an integer if decimal is zero. + + Returns: The output text string to be displayed.''' This docstring should be moved into the correct function. ------------------------------ In adafruit_ht16k33/segments.py <#53 (comment)> : > + + if self.debug: + print("(1) number = {0}, places = {1}, decimal = {2}, dot = {3}, stnum = '{4}'".format(number, places, decimal, dot, stnum)) + + if ((places <= 0) and (decimal > 0)): + self.fill(False) + places = 4 + + if '.' in stnum: + places += 1 + + if self.debug: + print("(2) places = {0}, dot = {1}, decimal = {2}, stnum = '{3}'".format(places, dot, decimal, stnum)) + + # Set decimal places, if number of decimal places is specified (decimal > 0) + if ((places > 0) and (decimal > 0) and (dot > 0) and (len(stnum[places:]) > decimal)): This is failing pylint because of all the parenthesis and variable > 0 can be shortened to variable. I'd suggest changing this line to: if places and decimal and dot and len(stnum[places:]) > decimal: ------------------------------ In adafruit_ht16k33/segments.py <#53 (comment)> : > + stnum = str(number) + dot = stnum.find('.') + + if ((len(stnum) > 5) or ((len(stnum) > 4) and (dot < 0))): + raise ValueError("Input overflow - {0} is too large for the display!".format(number)) + + if dot < 0: + # No decimal point (Integer) + places = len(stnum) + else: + places = len(stnum[:dot]) + + if self.debug: + print("(1) number = {0}, places = {1}, decimal = {2}, dot = {3}, stnum = '{4}'".format(number, places, decimal, dot, stnum)) + + if ((places <= 0) and (decimal > 0)): This is failing pylint because of the parenthesis and variable > 0 can be shortened to variable. I'd suggest changing this line to: if places <= 0 and decimal: — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#53?email_source=notifications&email_token=AA5DR24VAZDJHQANK5KNMU3Q7IHL3A5CNFSM4KKNPB2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCS4HA2A#pullrequestreview-347631720>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5DR2ZX2RLUNL2IPLWI4SLQ7IHL3ANCNFSM4KKNPB2A> .

[makermelissa]

Ok yeah, that makes sense. If pylint fails on that, you can add lines to disable certain checks with #pylint: disable-msg=[pylint-fail-code] and replace [pylint-fail-code] with the actual fail code.

[hybotix]

Done! I will get better as I learn what PyLint expects.

[makermelissa]

Just one more small thing with regards to some commented out duplicate code, but everything else looks good.

[makermelissa]

Just a minor nitpick, can we remove this commented out line please?

[hybotix]

Done! I would have nitpicked that too.

[makermelissa]

Looks great! Thanks