[ariaDownload.py] Code refactoring and adding progress bar

[ehavazli]

• Simplified and reorganized imports. Removed redundant imports and added specific modules (e.g., from datetime import datetime, timedelta)
• Set asf_search logging level to ERROR to suppress warning messages like:

2024-10-08 13:05:05,252 - asf_search - WARNING - Query Time Elapsed 0.9452362060546875
2024-10-08 13:05:05,261 - asf_search - WARNING - Product Subclassing Time 0.009154081344604492
2024-10-08 13:05:05,262 - asf_search - WARNING - Page Processing Time 6.9141387939453125e-06
2024-10-08 13:05:05,262 - asf_search - WARNING - Page Time Elapsed 0.9928610324859619

• Added type hints to functions and methods.
• Replaced old style strings with f-strings.
• Updated argument parsing
• Refactored the downloader and query into smaller methods for better readability.

Example Run Output
ariaDownload.py --bbox "34.6 34.8 -118.1 -117.9" --track 71 --output Download --start 20211214 --end 20220102

/Users/havazli/miniconda3/envs/ARIA-tools/bin/ariaDownload.py:4: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  __import__('pkg_resources').require('ARIAtools==1.2.1')
 
2024-10-08 13:20:01,158 - ariaDownload.py - INFO - Downloading 6 products...
Downloading: 100%|███████████████████████████████████████████████████████████████████████| 6/6 [06:24<00:00, 64.08s/file]
2024-10-08 13:26:25,668 - ariaDownload.py - INFO - Download complete. Wrote -- 6 -- products to: /Users/havazli/Desktop/ARIA_tests/products

[pep8speaks]

Hello @ehavazli! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-10-11 00:06:09 UTC

[sssangha]

LGTM! I made very minor tweak to the default start date to reflect the timing of the first NISAR beta products. Before an earlier scene was getting kicked out.

[alexfore]

I noticed a lot of changes which are not related to the addition of a progress bar.

IMO this pull request should be reduced to only the changes needed to implement the functionality and not include any style changes. I feel some of these changes make the code less explicit and more confusing. I could elaborate on specifics but I'm not sure that will be helpful.

Regarding imports:
I have removed the vast majority of from X import Y style from the rest of this code, I want to keep it this way with the full namespace used. (explicit > implicit)

Regarding logger messages:
The asf_search log level suppression should probably be moved to ARIAtools.util.log similar to other items in there. That will be executed at import time and suppress those messages for everything which imports that module.

Just as a general note to file away, best practice for logger messages is to use lazy formatting to optimize performance (won't format string unless the message will be written). So avoid f-strings and .format() calls in there but use additional arguments and %-style formatting:
https://stackoverflow.com/questions/66993387/use-lazy-formatting-in-logging-functions-pylint-error-message
https://medium.com/flowe-ita/logging-should-be-lazy-bc6ac9816906

I know this was already in there and you did not add this, but we should remove this line:

LOGGER.setLevel(logging.DEBUG if self.args.verbose else logging.INFO)

The global log level should only ever be set in the main() function and I should have deleted this before.

[ehavazli]

@alexfore

Regarding imports:
I have removed the vast majority of from X import Y style from the rest of this code, I want to keep it this way with the full namespace used. (explicit > implicit)

Is there a benefit to importing the whole module rather than the needed pieces? Importing the whole module will increase the memory usage.

Regarding logger messages: The asf_search log level suppression should probably be moved to ARIAtools.util.log similar to other items in there. That will be executed at import time and suppress those messages for everything which imports that module.

Moved the line to log.py

Just as a general note to file away, best practice for logger messages is to use lazy formatting to optimize performance (won't format string unless the message will be written). So avoid f-strings and .format() calls in there but use additional arguments and %-style formatting: https://stackoverflow.com/questions/66993387/use-lazy-formatting-in-logging-functions-pylint-error-message https://medium.com/flowe-ita/logging-should-be-lazy-bc6ac9816906

Switched f-string formatted logger messages to lazy formatting.

I know this was already in there and you did not add this, but we should remove this line:

LOGGER.setLevel(logging.DEBUG if self.args.verbose else logging.INFO)

The global log level should only ever be set in the main() function and I should have deleted this before.

Would you like to open a separate PR for this?

[alexfore]

Is there a benefit to importing the whole module rather than the needed pieces? Importing the whole module will increase the memory usage.

Yes, it is more explicit (zen of python) that is enough of a benefit to (nearly) always import the entire module. There will always be some exceptions where it makes sense but IMO not with basic module imports.

The entire module is imported either way so there is no compute time or memory savings in using from X import Y vs import X
https://softwareengineering.stackexchange.com/questions/187403/import-module-vs-from-module-import-function

Can the util.log module treat the asf_search log level setter identically as the others (I think it can be)? Then it can be just another item in the list of module names which have the log level set to logging.WARNING.

Finally, I didn't see any commits addressing or discussion of all of the unrelated changes included in the PR.