search

NeoGeographyToolkit / StereoPipeline

The NASA Ames Stereo Pipeline is a suite of automated geodesy & stereogrammetry tools designed for processing planetary imagery captured from orbiting and landed robotic explorers on other planets.
Apache License 2.0
478 stars 168 forks source link

Several Python scripts use Python2 `basestring` #321

Closed dpmayerUSGS closed 3 years ago

dpmayerUSGS commented 3 years ago

It appears that a few Python scripts in ASP use the basestring built-in abstract type. This was removed upon the transition from Python 2 to 3, and users are supposed to use str instead.

The use of basestring in parallel_stereo caused the script to fail with an error when I ran the following command:

parallel_stereo --nodes-list nodes.lis --threads-singleprocess 14 --entry-point 0 --stop-point 1 img1.map.cub img2.map.cub img3.map.cub -s stereo_map.config results/run_1

The error message is

Traceback (most recent call last):
  File "/scratch/dpmayer/ASP/ASP_latest/libexec/parallel_stereo", line 784, in <module>
    isinstance(option_dict[var], basestring)):
NameError: name 'basestring' is not defined

I'm running Python version 3.8.3. I've found the bug in both the latest 2.7.0 release and the daily build from 14 October 2020.

When I replace basestring with str on the following line, parallel_stereo runs as expected: https://github.com/NeoGeographyToolkit/StereoPipeline/blob/3a2b03cebd8373da37fa28cb2da4f7e177e8093a/src/asp/Tools/parallel_stereo#L784

A quick search reveals basestring is also present in asp_geo_utils.py and asp_string_utils.py. It looks like it occurs as part of a function to provide Python 2 and 3 compatibility in asp_string_utils.py, but the occurrence in asp_geo_utils.py could also throw an error.

I'm surprised I haven't run into this issue before running ASP and Python 3.

oleg-alexandrov commented 3 years ago

David, thanks. I put a fix in https://github.com/NeoGeographyToolkit/StereoPipeline/commit/4416fe53bb7fbeb246202d14889b1c5ddbbde3ae which will hopefully work with both Python 2 and 3. Seems to work based on a quick test, and if tonight's regression agrees, it will be part of tomorrow's build.