fiftysevendegreesofrad / sdna_plus

Fully open release with all advanced sDNA+ features
Other
10 stars 1 forks source link

Bug. TypeError: 'NoneType' object is not subscriptable. Unpin PyShp to fix in Cross_platform branch? #97

Open JamesParrott opened 1 month ago

JamesParrott commented 1 month ago

The main branch of sDNA has a static copy of PyShp 2.1.0 https://github.com/fiftysevendegreesofrad/sdna_plus/blob/729bd9824a7463fb2089fef3154bf6f6447a6e0a/arcscripts/shapefile.py#L9

The bug in the description comes from the older versions of PyShp 2.1.0, so can still occur in sDNA using these older versions of PyShp. PyShp fixed the bug in v2.1.3 (and is now on v2.3.1) https://github.com/GeospatialPython/pyshp/commit/f2f91ce172fc0170d0ef8b138e0140d2a041ea2e

In the cross platform branch, PyShp is pinned to 2.1.0 for strict compatibility.

https://github.com/fiftysevendegreesofrad/sdna_plus/blob/Cross_platform/requirements/base.txt

It's trivial to relax or bump the version pin. But the pin currently ensures the two branches use exactly the same code.

Unpinning the PyShp version is a design change towards dynamic linking instead of dynamic linking with version pinning (being reasonably close to the previous static linking). This would only affect the other branch though.

A quick scroll through the issues will show numerous other documented differences between the branches (in order to achieve compilation and passing of the vast majority of the regression tests). In comparison to those, shipping or allowing updated PyShp versions, is easy, harmless and best practise.

I've tested removing the version pin, in a new branch based on the Cross_Platform branch:

https://github.com/fiftysevendegreesofrad/sdna_plus/compare/Cross_platform...Latest_PyShp_(do_not_pin_to_v2.1.0)

No test failures occurred: https://github.com/fiftysevendegreesofrad/sdna_plus/actions?query=branch%3ALatest_PyShp_%28do_not_pin_to_v2.1.0%29++

The existing the tests didn't uncover this bug however.

JamesParrott commented 1 month ago

Candidate fix for the main branch too: https://github.com/fiftysevendegreesofrad/sdna_plus/tree/refs/heads/main_PyShp_2.3.1

Currently being built and tested: https://github.com/fiftysevendegreesofrad/sdna_plus/actions/runs/11270940170