Skip to content

Addresses DEM download issues#56

Merged
forrestfwilliams merged 4 commits intoMultiSAR:developfrom
Alex-Lewandowski:develop
Feb 21, 2026
Merged

Addresses DEM download issues#56
forrestfwilliams merged 4 commits intoMultiSAR:developfrom
Alex-Lewandowski:develop

Conversation

@Alex-Lewandowski
Copy link
Copy Markdown
Collaborator

Version 1.1 of the NISAR DEM was pulled and replaced with v1.2, which has updated endpoints.

This PR:

  • Updates the NISAR DEM endpoints in dem.py.
  • Adds some descriptive error handling to fetch.download_file() to enable easier debugging the next time expected data vanishes.

Comment thread src/multirtc/fetch.py Outdated
if chunk:
f.write(chunk)
bytes_written += len(chunk)
if bytes_written == 0:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey just looking at these lines. Are we sure the behavior we want is for the function to continue returning an empty string if the download fails? I'd prefer the function to fail if the download fails. Like the extra messaging though.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that makes sense. I'll replace those with raised exceptions.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I removed the check for zero bytes (probably unnecessary), replaced the empty-string return with a raise, and added logic to delete partially downloaded data if an exception is raised.

@forrestfwilliams
Copy link
Copy Markdown
Collaborator

Also will need to update the changelog.

@forrestfwilliams
Copy link
Copy Markdown
Collaborator

Also @Alex-Lewandowski there's a test for the nisar dem url that you'll need to update as well.

@Alex-Lewandowski
Copy link
Copy Markdown
Collaborator Author

Alex-Lewandowski commented Feb 11, 2026

@forrestfwilliams We received guidance that the DEM should be referred to as the "Modified Copernicus DEM for NISAR," so I updated references to the "OPERA DEM."

@Alex-Lewandowski
Copy link
Copy Markdown
Collaborator Author

Thanks for the review, @forrestfwilliams! You'll have to hit the merge button. I don't have write permission.

@forrestfwilliams forrestfwilliams merged commit dffbeb9 into MultiSAR:develop Feb 21, 2026
4 checks passed
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.

2 participants