Skip to content

Conversation

John-Ragland
Copy link
Member

I'm going to go ahead and create this PR and we can update it as we add more functionality

@John-Ragland
Copy link
Member Author

Hey @anishdixit-uw, would you mind testing out this script and see if it works for you?

@John-Ragland
Copy link
Member Author

I think the unit tests are failing because we need to add xarray as a dependancy

@anishdixit-uw
Copy link
Collaborator

Okay @John-Ragland , I'll try running the script locally

Copy link
Collaborator

@anishdixit-uw anishdixit-uw left a comment

Choose a reason for hiding this comment

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

Just had to install the tqdm library but the download works properly following the instructions to run download_hydrophone_data.py.

@John-Ragland
Copy link
Member Author

Just had to install the tqdm library but the download works properly following the instructions to run download_hydrophone_data.py.

Okay, I can add tqdm to the required packages. And after figuring out why pre-commit is failing, this will be ready to merge.

@anishdixit-uw
Copy link
Collaborator

@John-Ragland I figured the issue with pre-commit, it fails for flake8, which is just a styling check.

If you replace line 124:
filename = f'{args.output_path}/{hdata_ds.stats.location}_{hdata_ds.stats.starttime.strftime("%Y%m%dT%H%M%S")}_{hdata_ds.stats.endtime.strftime("%Y%m%dT%H%M%S")}'

With this block of code:

  op_path = args.output_path
  hdat_loc = hdata_ds.stats.location
  hdat_start_time = hdata_ds.stats.starttime.strftime("%Y%m%dT%H%M%S")
  hdat_end_time = hdata_ds.stats.endtime.strftime("%Y%m%dT%H%M%S")
  filename = f'{op_path}/{hdat_loc}_{hdat_start_time}_{hdat_end_time}'

It should now pass

@John-Ragland
Copy link
Member Author

Added your code block. I had thought that the pre-commit stuff fixed the styling automatically. For now I can just fix this line manually like you suggested.

Copy link
Collaborator

@anishdixit-uw anishdixit-uw left a comment

Choose a reason for hiding this comment

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

All checks passed

@John-Ragland
Copy link
Member Author

🎉

@John-Ragland John-Ragland merged commit 98c9a76 into 116_prerelease Jul 19, 2023
@John-Ragland John-Ragland deleted the download_scripts branch July 19, 2023 00:23
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