Skip to content

Fix listing dependencies in json #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

Neradoc
Copy link
Contributor

@Neradoc Neradoc commented Aug 18, 2022

By converting dependencies to _, the 1.10.2 release scrambled the requirements in the json file, moving all "dependencies" to "external dependencies" with an underscored pypi name instead of the library name (adafruit_circuitpython_hid instead of adafruit_hid).

This PR uses - not _ to reference modules from dependencies, so it matches the index used in package_list and the pypi_name field. Use the same function in both parts to emphasize their relatedness. The original name before transformation is still used in external_dependencies.

Note: this does not keep the same format as pipkin as intended by #90. pipkin can map the format function on the list to make sure every reference uses the same format without having to edit the list manually.

Additionally:

  • Add ~[; to filter requirements and strip() spaces. See here examples of valid formats.
  • Use sets to simplify the uniqueness test. Previously if line in dependencies would always be False. (testing the presence of adafruit-circuitpython-dotstar before adding adafruit_dotstar).
  • Add spidev to the list of skipped dependencies. (Found in the community bundle).
  • Sort the dependencies to make comparisons for that kind of check easier in the future. And use sort_keys in json, because why not.

With this PR the generated json matches the one generated by 1.10.1 except newly skipped libraries are skipped (pyasn1, pillow, spidev), and bad dependencies are fixed (typing-extensions~).

@tekktrik
Copy link
Member

I can try to take a look at this tonight or tomorrow, but adding others in case they can get to it first!

@tekktrik tekktrik requested a review from a team August 18, 2022 17:36
@Neradoc
Copy link
Contributor Author

Neradoc commented Aug 19, 2022

I believe this bug has killed the project bundler's ability to include dependencies, since it's not looking in the right list.
For example: https://learn.adafruit.com/smart-mirror-with-pyportal/coding-the-smart-mirror

lib
  adafruit_bitmap_font
  adafruit_display_text
  adafruit_pyportal

versus (installing adafruit_pyportal with circup)

lib
  adafruit_bitmap_font
  adafruit_bus_device
  adafruit_display_text
  adafruit_esp32spi
  adafruit_fakerequests.mpy
  adafruit_io
  adafruit_minimqtt
  adafruit_miniqr.mpy
  adafruit_pixelbuf.mpy
  adafruit_portalbase
  adafruit_pyportal
  adafruit_requests.mpy
  adafruit_touchscreen.mpy
  neopixel.mpy
  simpleio.mpy

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Approving immediately due to urgency.

@dhalbert dhalbert merged commit 0a2ac58 into adafruit:main Aug 19, 2022
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