Skip to content
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

s1_orbit.get_orbit_file_from_dir(): add auto_download arg #71

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

yunjunz
Copy link
Member

@yunjunz yunjunz commented Sep 26, 2022

  • s1_orbit.get_orbit_file_from_dir(): add auto_download arg to automatically download the orbit file if not found. The default is False.

  • remove the unused get_orbit_file_from_list()

  • utils.plot_bursts.py: switch to get_orbit_file_from_dir()

+ automatically download the orbit file if not found

+ remove the unused get_orbit_file_from_list()

+ utils.plot_bursts.py: switch to get_orbit_file_from_dir()
Copy link
Contributor

@scottstanie scottstanie left a comment

Choose a reason for hiding this comment

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

This looks good to me, and is great timing- I was thinking about how the orbit files should download if you've only got the .zip files, so thanks for this.

@@ -245,50 +267,23 @@ def get_orbit_file_from_list(zip_path: str, orbit_file_list: list[str]) -> str:
t_orbit_start = datetime.datetime.strptime(t_orbit_start[1:], FMT)

# string '.EOF' from end of end time string
t_orbit_end = datetime.datetime.strptime(t_orbit_end[:-4], FMT)
t_orbit_stop = datetime.datetime.strptime(t_orbit_end[:-4], FMT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just checking- this name change was to make line 275 more logical (since the comment above there has "stop" instead of "end")?
I'm noticing now that this file alone has 26 instances of end_* and 11 instances of stop_*, and most other files have a combo of "stop" and "end". Not sure if we want to pick one and change the rest to match, but perhaps that can be another pull request after this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer stop rather than end, because 1) "stopTime" is used in the S1 XML file, and 2) isce2 variable names, and 3) burst member variable here.

Replacing "end" with "stop" throughout s1reader sounds good to me. And yes, I think it's better to do that in another dedicated PR.

Copy link
Contributor

@vbrancat vbrancat left a comment

Choose a reason for hiding this comment

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

LGTM

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