[Bug]: `ET.parse(orbit_path)` FileNotFoundError

[yunjunz]

Checked for duplicates

Yes - I've already checked. Found by Ozzy Tirmizi.

Describe the bug

>>> bursts = s1reader.load_bursts(zip_path, orbit_path, swath_num=2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/yunjunz/tools/s1-reader/src/s1reader/s1_reader.py", line 476, in load_bursts
    bursts = _burst_from_zip(path, id_str, orbit_path)
  File "/Users/yunjunz/tools/s1-reader/src/s1reader/s1_reader.py", line 536, in _burst_from_zip
    bursts = burst_from_xml(f_annotation, orbit_path, f_tiff, iw2_f_annotation, z_file.open)
  File "/Users/yunjunz/tools/s1-reader/src/s1reader/s1_reader.py", line 339, in burst_from_xml
    orbit_tree = ET.parse(orbit_path)
  File "/Users/yunjunz/tools/miniconda3/envs/opera/lib/python3.10/xml/etree/ElementTree.py", line 1229, in parse
    tree.parse(source, parser)
  File "/Users/yunjunz/tools/miniconda3/envs/opera/lib/python3.10/xml/etree/ElementTree.py", line 569, in parse
    source = open(source, "rb")
FileNotFoundError: [Errno 2] No such file or directory: ''
>>> 

What did you expect?

I expected [...]

Reproducible steps

import s1reader
zip_file = './SLC/S1A_IW_SLC__1SSV_20160829T230628_20160829T230655_012821_014394_A1F0.zip'
orbit_dir = './orbits'
orbit_path = s1reader.get_orbit_file_from_dir(zip_path, orbit_dir)
bursts = s1reader.load_bursts(zip_path, orbit_path, swath_num=2)

Environment

- Version of this software [e.g. vX.Y.Z]
- Operating System: [e.g. MacOSX with Docker Desktop vX.Y]
...

[yunjunz]

The issue I had is not finding the orbit file. After downloading it manually using isce2, the error is gone.

[yunjunz]

We should print out a warning msg or raise error if s1reader.get_orbit_file_from_dir() returns empty.

[hfattahi]

Yes I agree we should give a meaningful error message.

[LiangJYu]

#46 partially addresses this issue.

s1_reader.load_bursts lacks any checks for orbit_path. orbit_path is blindly passed forward regardless of SAFE type.

If an os.path.isfile(orbit_path) is added before bursts are loaded, FileNotFoundError would be raised earlier. Unfortunately, burst loading will not actually know if orbit_path is valid XML until all the annotation XML is read here.

Discounting #41, passing orbit as an XML element tree to burst loading method would ensure that a XML is passed to a burst loading method. Another layer error checking, would be to convert the XML to a list of state vectors to ensure a valid XML orbit file was passed into load_bursts.

What do you all think?

[yunjunz]

Adding the os.path.isfile(orbit_file) sounds good to me.

Since load_bursts() will be frequently used, I like the current passing of orbit_file, instead of an XML or list object. To be fair, we also do not validate the SAFE annotation files until laterward, so I don't see the reason to do it for the orbit file beforehand.