search

adafruit / Adafruit_CircuitPython_CircuitPlayground

CircuitPython library for Circuit Playground Express
MIT License
91 stars 71 forks source link

Annotation fixes in circuitplayground_base and bluefruit.py #118

Closed tcfranks closed 2 years ago

tcfranks commented 2 years ago

LMK how this looks....

tcfranks commented 2 years ago

wow, that was a lot to take in. pylint is throwing complaints about line endings, but the last big one it's choking on is: adafruit_circuitplayground\circuit_playground_base.py:149:26: E1101: Module 'os' has no 'uname' member; maybe 'name'? (no-member) which refers to this line: if ( "nRF52840" in os.uname().machine

Clearly uname() returns a namedtuple that includes machine, so why is pylint kicking this out? Any suggestions?

tekktrik commented 2 years ago

CPython uses os.name but CircuitPython uses os.uname. We could disable it at that point, but I wonder if pylint stops complaining once the typing stubs are imported (something the CI may do).

When you push code, git should be fixing the line endings for you. They are different between Linux and Windows, so my guess is that you're developing on windows where the line ending is CRLF, but git prefers LF. If you push the code, it should get automatically resolved, and then I can give it another review, as well as see whether that remaining pylint error goes away.

Neradoc commented 2 years ago

CPython uses os.name but CircuitPython uses os.uname. We could disable it at that point, but I wonder if pylint stops complaining once the typing stubs are imported (something the CI may do).

Note that os.uname is C python: https://docs.python.org/3/library/os.html#os.uname It's a little weird though, it's not available on windows, only Unix, so that might be the issue here. Or pylint is being weird.

tekktrik commented 2 years ago

Oh neat, good point, and thanks for that correction! I think pylint can also lint based on your version of Python (won't flag things if it was added in a later version of Python), so it may be something similar with regards to the operating system.

It sounds like this might just be a good time to push the code to this branch and see what still remains to be done. We can even give those suggestions as comments to the lines.

tekktrik commented 2 years ago

Looks like you'll need to run pre-commit, fix setup.py, and revert to using those tick marks!

tcfranks commented 2 years ago

hmmm black didn't like requirements.txt the process didn't like the way black changed it. I put the important bits back they way they were and re-checked it and apparently all is good. LMK

tcfranks commented 2 years ago

also, I run pylint and black manually. I downloaded and installed precommit but for some reason on my windows box it won't run as a unit, in spite of having looked at path, etc. any ideas, besides running linux? :)

tcfranks commented 2 years ago

oh duh. well - I think I just solved the pre-commit problem.....we'll see next time I need to use it.