adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.07k stars 1.2k forks source link

tools/test-stub.sh broken. Only one of two tests actually fails #8891

Closed tannewt closed 7 months ago

tannewt commented 8 months ago

mypy -c 'import busio; b: busio.I2C; b.readfrom_into(0x30, b"")'

Is supposed to fail because readfrom_into() needs to write into the buffer but is given a byte string. However, it doesn't. WriteableBuffer includes two string types that break the whole thing. If you remove them, then the test will fail.

Running mypy on the typing module gives a clue in how it fails:

mypy circuitpython_typing
circuitpython_typing/http.py:16: error: Skipping analyzing "adafruit_requests": module is installed, but missing library stubs or py.typed marker  [import-untyped]
circuitpython_typing/device_drivers.py:12: error: Skipping analyzing "adafruit_bus_device.i2c_device": module is installed, but missing library stubs or py.typed marker  [import-untyped]
circuitpython_typing/device_drivers.py:12: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
circuitpython_typing/__init__.py:39: error: Name "rgbmatrix" is not defined  [name-defined]
circuitpython_typing/__init__.py:40: error: Name "ulab" is not defined  [name-defined]
circuitpython_typing/__init__.py:48: error: Name "rgbmatrix" is not defined  [name-defined]
circuitpython_typing/__init__.py:49: error: Name "ulab" is not defined  [name-defined]
circuitpython_typing/__init__.py:131: error: Name "audiocore" is not defined  [name-defined]
circuitpython_typing/__init__.py:132: error: Name "audiocore" is not defined  [name-defined]
circuitpython_typing/__init__.py:133: error: Name "audiomixer" is not defined  [name-defined]
circuitpython_typing/__init__.py:134: error: Name "audiomp3" is not defined  [name-defined]
circuitpython_typing/__init__.py:135: error: Name "synthio" is not defined  [name-defined]
circuitpython_typing/__init__.py:141: error: Name "rgbmatrix" is not defined  [name-defined]
circuitpython_typing/__init__.py:144: error: Name "alarm" is not defined  [name-defined]
circuitpython_typing/socket.py:129: error: Name "_FakeSSLContext" is not defined  [name-defined]
Found 14 errors in 4 files (checked 8 source files)

It seems we have a circular dependency between circuitpython_typing and circuitpython stubs.

justmobilize commented 8 months ago

Did this come from the stub work, or is it unrelated?

justmobilize commented 8 months ago

@tannewt,

I was able to get this to pass by:

  1. Adding the following imports to circuitpython_typing/__init__.py:
import alarm
import audiocore
import audiomixer
import audiomp3
import rgbmatrix
import synthio
import ulab
from alarm.pin import PinAlarm
from alarm.time import TimeAlarm
from ulab.numpy import ndarray
  1. Adding the following imports to circuitpython_typing/socket.py:
from adafruit_requests import _FakeSSLContext

or

class _FakeSSLContext:
    """Some description here"""

Note: this will move into ConnectionManager, so you could either get that merged first and make the needed updates, or do the second option and just define _FakeSSLContext

  1. in circuitpython-stubs/synthio/__init__.pyi I got the following errors:
circuitpython-stubs/synthio/__init__.pyi:151: error: Incompatible default for argument "waveform" (default has type "None", argument has type "array[Any] | bytearray | bytes | memoryview | RGBMatrix | ndarray")  [assignment]
circuitpython-stubs/synthio/__init__.pyi:151: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
circuitpython-stubs/synthio/__init__.pyi:151: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
circuitpython-stubs/synthio/__init__.pyi:346: error: Incompatible default for argument "ring_waveform" (default has type "float", argument has type "array[Any] | bytearray | bytes | memoryview | RGBMatrix | ndarray | None")  [assignment]

So in circuitpython-stubs/synthio/__init__.pyi:151 I removed the default and in circuitpython-stubs/synthio/__init__.pyi I changed 0.0 to None

  1. Instead of:
pip install --force-reinstall circuitpython-stubs/dist/circuitpython-stubs-*.tar.gz

I did:

export MYPYPATH=circuitpython-stubs/
justmobilize commented 8 months ago

ugh... Made a branch for adafruit-circuitpython-typing, but then the circular dependencies block the install. Will keep looking

justmobilize commented 8 months ago

two test branches for you: https://github.com/adafruit/circuitpython/compare/main...justmobilize:circuitpython:typing-fixes?expand=1 https://github.com/adafruit/Adafruit_CircuitPython_Typing/compare/main...justmobilize:Adafruit_CircuitPython_Typing:typing-fixes?expand=1

image

I know the try/except to get around the circular imports is strange, but not sure another way...

dhalbert commented 8 months ago

It seems we have a circular dependency between circuitpython_typing and circuitpython stubs.

It seems like this would come up in the CPython world too. I wonder how they get around it.

justmobilize commented 8 months ago

Fixed the try/except by using if TYPE_CHECKING. Which looking is what seems to be common

justmobilize commented 8 months ago

@tannewt, 2 questions:

  1. Where would you like _FakeSSLContext imported from in circuitpython_typing/socket.py? We can either:

    • Do adafruit_requests now and if/when adafruit_connection_manager is merged, update it (before we remove it from requests)
    • Wait for adafruit_connection_manager
  2. Looking at the change for synthio, do you know what should be in the pyi files?

jepler commented 8 months ago

I filed #8895 for the synthio problems (I don't get why we weren't seeing this at CP build time!) but if you want to cover it in some other PR that's fine. The waveform argument should be Optional and the correct default is None.

tannewt commented 8 months ago

@tannewt, 2 questions:

1. Where would you like `_FakeSSLContext` imported from in `circuitpython_typing/socket.py`? We can either:

   * Do `adafruit_requests` now and if/when `adafruit_connection_manager` is merged, update it (before we remove it from requests)
   * Wait for `adafruit_connection_manager`

2. Looking at the change for [synthio](https://github.com/adafruit/circuitpython/compare/main...justmobilize:circuitpython:typing-fixes?expand=1), do you know what should be in the pyi files?

Lets wait for connection manager. The retry bug is exactly why we should factor out that code. (So we can fix it in one place.)

justmobilize commented 8 months ago

Sounds good

justmobilize commented 8 months ago

@tannewt if you look at this PR for Adafruit_CircuitPython_Typing, if I pull out the changes to circuitpython_typing/socket.py and requirements.txt would you be good with that? It will get almost everything done.

justmobilize commented 8 months ago

@tannewt OMG. Realizing that circuitpython_typing/socket.py exists, means that we don't need that defined in ConnectionManager code at all. And that kills a whole kb of the mpy. Technically we could also remove it from Requests now...

If we actually add a _FakeSSLContext definition in circuitpython_typing/socket.py, we can fix this bug now and remove all that typing code from ConnectionManager

dhalbert commented 8 months ago

@justmobilize Oh, please, certainly, do that!

justmobilize commented 8 months ago

It's been a while since I got excited for removing a KB of compiled code...

So here's the PR, I'll take it out of draft as soon as the mpy thing get's figured out

justmobilize commented 8 months ago

@tannewt PR up - https://github.com/adafruit/circuitpython/pull/8898