Skip to content

fix Pimoroni LTR559 is not a known CircuitPython library#120

Merged
askpatrickw merged 1 commit into
adafruit:mainfrom
dgriswo:fix_pimoroni_ltr559
Aug 20, 2021
Merged

fix Pimoroni LTR559 is not a known CircuitPython library#120
askpatrickw merged 1 commit into
adafruit:mainfrom
dgriswo:fix_pimoroni_ltr559

Conversation

@dgriswo
Copy link
Copy Markdown
Contributor

@dgriswo dgriswo commented Aug 17, 2021

added Pimoroni LTR559 library to the not_standard_names dictionary.

user@machine:~/repos/circup $ python3 ./circup.py --path /tmp/circuitpython show | grep ltr559
pimoroni_circuitpython_ltr559
user@machine:~/repos/circup $ python3 ./circup.py --path /tmp/circuitpython install pimoroni_circuitpython_ltr559
Found device at /tmp/circuitpython, running CircuitPython 6.3.0.
Searching for dependencies for: ['pimoroni_circuitpython_ltr559']
WARNING:
	pimoroni_ltr559 is not a known CircuitPython library.
Ready to install: []

Post addition:

user@machine:~/repos/circup $ python3 ./circup.py --path /tmp/circuitpython install pimoroni_circuitpython_ltr559
Found device at /tmp/circuitpython, running CircuitPython 6.3.0.
Searching for dependencies for: ['pimoroni_circuitpython_ltr559']
Ready to install: ['adafruit_bus_device', 'adafruit_register', 'pimoroni_circuitpython_ltr559']

Installed 'adafruit_bus_device'.
Installed 'adafruit_register'.
Installed 'pimoroni_circuitpython_ltr559'.

@tannewt tannewt requested a review from askpatrickw August 18, 2021 17:42
@askpatrickw
Copy link
Copy Markdown
Contributor

I thought this looked straight forward, but as I look at this, I see the repo name https://github.com/pimoroni/Pimoroni_CircuitPython_LTR559 and the .py file name and the setup.py all say Pimoroni_CircuitPython_LTR559. So I'm now confused as to why it would need to be special cased...

I'm probably forgetting something... digging ...

@Neradoc
Copy link
Copy Markdown
Contributor

Neradoc commented Aug 19, 2021

It's because pimoroni didn't drop _circuitpython_ from the actual module file name, like Adafruit libraries do.

clean_library_name, who's purpose is to convert pypi names and repository names to module file names, removes [-_]circuitpython[-_] from any name passed to it. A more general solution might be to test both before and after clean_library_name, in case the provided name is already the module name that happens to resemble a pypi/repo name, as is the case here.

In general though the conversion from pypi/module name only works as long as the name follows the exact same convention of prefix-circuitpython-thingamabob -> prefix_thingamabob[.py], and that might be a lot to ask of future 3rd party modules and bundles, as we can see. A better solution would be to implement the move to the json file, which would allow a reverse search from the pypi_name field or the repository name for each module.

@askpatrickw
Copy link
Copy Markdown
Contributor

It is a little more complicated 12 of the 41 community bundle libraries don't install.

Also, circuitpython_mylibrary and prefix_circtuitpython_mylibrary should work. They're actually the expected naming convention if you're using the library cookiecutter. Adafruit libraries dropping the cp is for a small number of very common but also older libraries, new ones don't do that.

I'll look at this more tonight... day job calls. ;-)

@Neradoc
Copy link
Copy Markdown
Contributor

Neradoc commented Aug 19, 2021

It is a little more complicated 12 of the 41 community bundle libraries don't install.

I don't see that. Outside of a bunch of C Python libraries that are in the requirements for jepler_udecimal I only see:

WARNING:
	pimoroni_ltr559 is not a known CircuitPython library.

Also, circuitpython_mylibrary and prefix_circtuitpython_mylibrary should work. They're actually the expected naming convention if you're using the library cookiecutter. Adafruit libraries dropping the cp is for a small number of very common but also older libraries, new ones don't do that.

Oh I didn't think of the cokie cutter allowing for no-prefix.

@askpatrickw
Copy link
Copy Markdown
Contributor

data ...

Total Libraries Tested: 41

11 Failed libraries: ['circuithon_base64', 'circuithon_display_frame', 'circuithon_hmac', 'nonblocking_timersparkfun_qwiicrelay', 'circuithon_nrf24l01gamblor21_ahrsodt_at42qt1070', 'barbudor_ina3221circuithon_parse', 'pimoroni_circuithon_ltr559', 'circuithon_schedule', 'bluepad32dotstar_featherwingi2cencoderlibv21', 'btevedynamixel', 'jepler_udecimalsparkfun_qwiicas3935']

1 Libraries with failed dependencies: [{'arrowline': 'vectorio'}]

@Neradoc
Copy link
Copy Markdown
Contributor

Neradoc commented Aug 19, 2021

circuithon ?

circup on  main [$] via 🐍 v3.9.6 (venv) took 2s 
❯ circup install adafruit_discordbot adafruit_soundboard arrowline azurecustomvision_prediction barbudor_ina3221 barbudor_tmp75 biffobear_as3935 bluepad32 bteve candlesticks circuitpython_base64 circuitpython_display_frame circuitpython_hmac circuitpython_nrf24l01 circuitpython_parse circuitpython_schedule community_tca9555 dotstar_featherwing dynamixel electronutlabs_ili9163 electronutlabs_lis2dh12 electronutlabs_ltr329als01 example gamblor21_ahrs gc9a01 hcsr04 i2c_button i2cencoderlibv21 ifttt jepler_udecimal mitutoyo morsecode nonblocking_timer odt_at42qt1070 pimoroni_circuitpython_ltr559 pimoroni_mics6814 piper_blockly sh1106 slight_tlc5957 sparkfun_qwiicas3935 sparkfun_qwiicjoystick sparkfun_qwiickeypad sparkfun_qwiicrelay sparkfun_qwiictwist sparkfun_serlcd styles trellism4_extended wiichuck wws_74hc165
Found device at /Volumes/CIRCUITPY, running CircuitPython 7.0.0-alpha.6-85-g5b0009cbc.
Searching for dependencies for: ['adafruit_discordbot', 'adafruit_soundboard', 'arrowline', 'azurecustomvision_prediction', 'barbudor_ina3221', 'barbudor_tmp75', 'biffobear_as3935', 'bluepad32', 'bteve', 'candlesticks', 'circuitpython_base64', 'circuitpython_display_frame', 'circuitpython_hmac', 'circuitpython_nrf24l01', 'circuitpython_parse', 'circuitpython_schedule', 'community_tca9555', 'dotstar_featherwing', 'dynamixel', 'electronutlabs_ili9163', 'electronutlabs_lis2dh12', 'electronutlabs_ltr329als01', 'example', 'gamblor21_ahrs', 'gc9a01', 'hcsr04', 'i2c_button', 'i2cencoderlibv21', 'ifttt', 'jepler_udecimal', 'mitutoyo', 'morsecode', 'nonblocking_timer', 'odt_at42qt1070', 'pimoroni_circuitpython_ltr559', 'pimoroni_mics6814', 'piper_blockly', 'sh1106', 'slight_tlc5957', 'sparkfun_qwiicas3935', 'sparkfun_qwiicjoystick', 'sparkfun_qwiickeypad', 'sparkfun_qwiicrelay', 'sparkfun_qwiictwist', 'sparkfun_serlcd', 'styles', 'trellism4_extended', 'wiichuck', 'wws_74hc165']
WARNING:
	pimoroni_ltr559 is not a known CircuitPython library.
WARNING:
	vectorio is not a known CircuitPython library.
WARNING:
	astroid is not a known CircuitPython library.
WARNING:
	black is not a known CircuitPython library.
WARNING:
	isort is not a known CircuitPython library.
WARNING:
	pre-commit is not a known CircuitPython library.
WARNING:
	recommonmark is not a known CircuitPython library.
WARNING:
	sphinx is not a known CircuitPython library.
WARNING:
	sphinx-autoapi is not a known CircuitPython library.
WARNING:
	sphinxcontrib-svg2pdfconverter is not a known CircuitPython library.
WARNING:
	sphinx-rtd-theme is not a known CircuitPython library.
WARNING:
	setuptools is not a known CircuitPython library.
WARNING:
	wheel is not a known CircuitPython library.
WARNING:
	twine is not a known CircuitPython library.
WARNING:
	readme_renderer is not a known CircuitPython library.
WARNING:
	testresources is not a known CircuitPython library.
WARNING:
	circuitpython_build_tools is not a known CircuitPython library.
Ready to install: ['adafruit_binascii', 'adafruit_bus_device', 'adafruit_datetime', 'adafruit_discordbot', 'adafruit_display_shapes', 'adafruit_display_text', 'adafruit_dotstar', 'adafruit_esp32spi', 'adafruit_hashlib', 'adafruit_matrixkeypad', 'adafruit_pixelbuf', 'adafruit_register', 'adafruit_requests', 'adafruit_seesaw', 'adafruit_soundboard', 'arrowline', 'azurecustomvision_prediction', 'barbudor_ina3221', 'barbudor_tmp75', 'biffobear_as3935', 'bluepad32', 'bteve', 'candlesticks', 'circuitpython_base64', 'circuitpython_display_frame', 'circuitpython_hmac', 'circuitpython_nrf24l01', 'circuitpython_parse', 'circuitpython_schedule', 'community_tca9555', 'dotstar_featherwing', 'dynamixel', 'electronutlabs_ili9163', 'electronutlabs_lis2dh12', 'electronutlabs_ltr329als01', 'example', 'gamblor21_ahrs', 'gc9a01', 'hcsr04', 'i2c_button', 'i2cencoderlibv21', 'ifttt', 'jepler_udecimal', 'mitutoyo', 'morsecode', 'neopixel', 'nonblocking_timer', 'odt_at42qt1070', 'pimoroni_mics6814', 'piper_blockly', 'sh1106', 'slight_tlc5957', 'sparkfun_qwiicas3935', 'sparkfun_qwiicjoystick', 'sparkfun_qwiickeypad', 'sparkfun_qwiicrelay', 'sparkfun_qwiictwist', 'sparkfun_serlcd', 'styles', 'trellism4_extended', 'wiichuck', 'wws_74hc165']

Installed 'adafruit_binascii'.
Installed 'adafruit_bus_device'.
Installed 'adafruit_datetime'.
Installed 'adafruit_discordbot'.
Installed 'adafruit_display_shapes'.
Installed 'adafruit_display_text'.
Installed 'adafruit_dotstar'.
Installed 'adafruit_esp32spi'.
Installed 'adafruit_hashlib'.
Installed 'adafruit_matrixkeypad'.
Installed 'adafruit_pixelbuf'.
Installed 'adafruit_register'.
Installed 'adafruit_requests'.
Installed 'adafruit_seesaw'.
Installed 'adafruit_soundboard'.
Installed 'arrowline'.
Installed 'azurecustomvision_prediction'.
Installed 'barbudor_ina3221'.
Installed 'barbudor_tmp75'.
Installed 'biffobear_as3935'.
Installed 'bluepad32'.
Installed 'bteve'.
Installed 'candlesticks'.
Installed 'circuitpython_base64'.
Installed 'circuitpython_display_frame'.
Installed 'circuitpython_hmac'.
Installed 'circuitpython_nrf24l01'.
Installed 'circuitpython_parse'.
Installed 'circuitpython_schedule'.
Installed 'community_tca9555'.
Installed 'dotstar_featherwing'.
Installed 'dynamixel'.
Installed 'electronutlabs_ili9163'.
Installed 'electronutlabs_lis2dh12'.
Installed 'electronutlabs_ltr329als01'.
Installed 'example'.
Installed 'gamblor21_ahrs'.
Installed 'gc9a01'.
Installed 'hcsr04'.
Installed 'i2c_button'.
Installed 'i2cencoderlibv21'.
Installed 'ifttt'.
Installed 'jepler_udecimal'.
Installed 'mitutoyo'.
Installed 'morsecode'.
Installed 'neopixel'.
Installed 'nonblocking_timer'.
Installed 'odt_at42qt1070'.
Installed 'pimoroni_mics6814'.
Installed 'piper_blockly'.
Installed 'sh1106'.
Installed 'slight_tlc5957'.
Installed 'sparkfun_qwiicas3935'.
Installed 'sparkfun_qwiicjoystick'.
Installed 'sparkfun_qwiickeypad'.
Installed 'sparkfun_qwiicrelay'.
Installed 'sparkfun_qwiictwist'.
Installed 'sparkfun_serlcd'.
Installed 'styles'.
Installed 'trellism4_extended'.
Installed 'wiichuck'.
Installed 'wws_74hc165'.

Note: I generated the list from a command in my upcoming PR to manage 3rd party bundles (circup bundle-info --modules) but also ls from inside the requirements directory. I suspect you removed ".py" from the list of module files with a regex-compatible tool without escaping the . (happens to me all the time).

@Neradoc
Copy link
Copy Markdown
Contributor

Neradoc commented Aug 19, 2021

On the other hand, while I am testing all the installs, the module adafruit-circuitpython-display-button required by adafruit_pyoa is named adafruit_button and it needs to go into the not_standard_names list:

        "adafruit_display_button": "adafruit_button",

@askpatrickw
Copy link
Copy Markdown
Contributor

OK... my test was wrong... I took all the .py filenames and then used that as the library name... I change my test to use circup show to generate the list and things look much better.

Total Libraries Tested: 333

0 Failed libraries: []

4 Libraries with failed dependencies: [
    {'adafruit_pyoa': 'adafruit_display_button'},
    {'arrowline': 'vectorio'},
    {'jepler_udecimal': 'astroid'},
    {'pimoroni_circuitpython_ltr559': 'pimoroni_ltr559'}
]

In reality, the pimoroni_circuitpython_ltr559 failure is not a dependency, but my check is just for if the failed library name is the same as one I asked circup to install. so now we're back to the core issue you originally identified.

Anyway.. I still don't get why this is needing to be special cased. I think this is a bug in circup.
I'm going to hop over to circuitpython-dev and see if I can get another set of eyes.

@askpatrickw
Copy link
Copy Markdown
Contributor

Ok.. I get it. The CP should not be in the filename.
And I verified this is true with cookiecutter generated libraries.

Since this is an older library, let's approve the PR.

Sorry for the long back and forth.

Copy link
Copy Markdown
Contributor

@askpatrickw askpatrickw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested and approved and discussed (a lot) :-)

@askpatrickw askpatrickw merged commit 350f1ce into adafruit:main Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants