Add unit tests for gpm_api.io #8

evanjt · 2023-07-19T17:17:12Z

Prework

I understand and agree to this repository's code of conduct.
I understand and agree to this repository's contributing guidelines.
I have already submitted an issue or discussion thread to discuss my idea with the maintainers.

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

Yes
No

If yes, please describe the impact and communicate accordingly:

The PR fulfills these requirements:

It's submitted to the branch named as follow :
- Fix a bug: bugfix-<some_key>-<word>
- Improve the doc: doc-<some_key>-<word>
- Improve a tutorial tutorial-<some_key>-<word>
- Add a new feature: feature-<some_key>-<word>
- Refactor some code: refactor-<some_key>-<word>
- Optimize some code: optimize-<some_key>-<word>
When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
Don't forget to link PR to issue if you are solving one.
All tests are passing.
New/updated tests are included

If adding a new feature, the PR's description includes:

A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

Related GitHub issues and pull requests

Ref: #

Summary

Please explain the purpose and scope of your contribution.

Incorporate tests for gpm_api.io

gpm_api/tests/io/test_checks.py

ghiggi

The tests looks great. I have just a couple of comments ;)

gpm_api/tests/io/test_checks.py

…uture use, allow check_time to fix incoming str and np objects into check_date()

sphamba

@evanjt Check why tests do not pass CI (Python 3.8 apparently)

gpm_api/tests/io/test_checks.py

gpm_api/io/checks.py

…s to not replace host (arthurhouftps.pps.eosdis.nasa.gov -> arthurhouftp.pps.eosdis.nasa.gov)

codecov · 2023-07-24T13:11:42Z

Codecov Report

Merging #8 (9bac278) into main (9e0212e) will decrease coverage by 0.16%.
Report is 96 commits behind head on main.
The diff coverage is 0.00%.

❗ Current head 9bac278 differs from pull request most recent head 03d7578. Consider uploading reports for the commit 03d7578 to get more accurate results

@@           Coverage Diff            @@
##            main      #8      +/-   ##
========================================
- Coverage   0.15%   0.00%   -0.16%     
========================================
  Files         41      40       -1     
  Lines       3300    3313      +13     
========================================
- Hits           5       0       -5     
- Misses      3295    3313      +18

Files Changed	Coverage Δ
gpm_api/io/checks.py	`0.00% <0.00%> (ø)`
gpm_api/io/directories.py	`0.00% <ø> (ø)`
gpm_api/io/download.py	`0.00% <0.00%> (ø)`
gpm_api/tests/conftest.py	`0.00% <0.00%> (ø)`

... and 1 file with indirect coverage changes

evanjt · 2023-07-24T13:33:59Z

@evanjt Check why tests do not pass CI (Python 3.8 apparently)

@sphamba They are not passing as type hints became supported with standard objects rather than needing to import them from the typing library in 3.9 https://peps.python.org/pep-0585/.

We can consider importing annotations from __future__ https://stackoverflow.com/questions/63939138/is-there-a-way-to-use-python-3-9-type-hinting-in-its-previous-versions in the Python 3.8 tests somehow, what do you think?

sphamba · 2023-07-24T14:54:41Z

@evanjt Check why tests do not pass CI (Python 3.8 apparently)

@sphamba They are not passing as type hints became supported with standard objects rather than needing to import them from the typing library in 3.9 https://peps.python.org/pep-0585/.

We can consider importing annotations from __future__ https://stackoverflow.com/questions/63939138/is-there-a-way-to-use-python-3-9-type-hinting-in-its-previous-versions in the Python 3.8 tests somehow, what do you think?

@ghiggi We could switch to the Python 3.8 style of type hinting (with from typing import ...) as in DisdroDB. Is that ok for you?

ghiggi · 2023-07-24T20:16:00Z

Or can we drop support for python 3.8 @sphamba @evanjt ?
It may be easier ....

sphamba · 2023-07-25T07:47:39Z

Or can we drop support for python 3.8 @sphamba @evanjt ? It may be easier ....

I don't know
Since we instruct users how to setup a virtual environment, we could very well set Python 3.9 as a minimum version, imo

evanjt · 2023-07-25T08:55:37Z

I think changing the type hints to those compatible with 3.8 is a good balance between improving typing for future versions and supporting older Python installations, especially after seeing the amount of people using 3.8 still. If we consider adding them to the library, then the application will not run at all for <= 3.8 versions.

gpm_api/tests/io/test_directories.py

evanjt · 2023-08-18T00:50:10Z

At this stage, I believe most of the testing is done in this module without spending a considerable amount of time mocking requests and responses

gpm_api/tests/io/test_filter.py

…lter_by_time(), modify the default None behaviour to choose utcnow() instead of now() to avoid timezone related issues

ghiggi

I reviewed a first part

gpm_api/io/checks.py

gpm_api/io/download.py

gpm_api/tests/conftest.py

gpm_api/tests/io/test_checks.py

gpm_api/tests/io/test_data_integrity.py

gpm_api/tests/io/test_directories.py

ghiggi

I finished to review the rest of the code. @evanjt we can have a Zoom in the coming days to clarify what remains to be done ;)
Have a nice week !

gpm_api/tests/io/test_download.py

ghiggi · 2023-08-29T12:56:04Z

gpm_api/tests/io/test_download.py

+) -> None:
+    """Test check_download_status function"""
+
+    for product in products:


Here we don't need to loop over products. product is an argument of _check_download_status just to display an informative message, but does not condition execution. So define a dummy product string within the test.

If the function changes in the future to be restricted to a product for whichever reason, should we not keep the loop in to check all possible products work with it as defined in the internal product list?

If it's a case of simplifying the test routine, I'm happy to simplify it, specifying a random string here would restrict the tests to a static constant outside of the scope of the realistic product list, perhaps we could just use the first element of the list?

Ok I agree. In the end I guess does not take lot of time to execute this test.

gpm_api/tests/io/test_download.py

gpm_api/tests/io/test_filter.py

gpm_api/tests/io/test_disk.py

…the mock_configuration fixture

regislon · 2023-09-05T13:18:19Z

Dear @ghiggi ,

This pull request has been open for more than one month. Can we close it without further changes and create new issues to address any important remarks you may have for the rest of the project? Keeping a pull request open for such a long time is not ideal in terms of traceability.

Furthermore, the time spent on this task is at the expense of others. Could we be picky at the end of the project ?

ghiggi · 2023-09-05T14:51:32Z

Hey @regislon and @evanjt !
I was expecting @evanjt to address some of the issues ... I didn't receive any message from him.
As it is, some of the changes to the codebase (datetime modified in ms instead of s) break the API.
I listed tasks that can be postponed (resolving the conversations above) into: #13
I recommend @evanjt to look at the 5-6 remaining unresolved conversations in all my reviews and address those to merge the PR. It should take max 1 hour... are just small fixes mainly related to dealing with the time objects!

gpm_api/tests/io/test_checks.py

… UTC and without

… as to not flatten an already flattened list

evanjt · 2023-09-06T07:14:22Z

Hi @ghiggi thanks for the comments. I replied to some of your messages and made changes last week but not all of them as my time was limited. Furthermore, as I understood from our last meeting, you did not want more hours spent on this module, so with the time I had, I limited it to the ones that did not require additional modifications of the code base.

With respect to your latest comments, the issue with the microseconds to seconds was a misjudgement of mine through the threads in this PR earlier in July, and have corrected it, along with the other items you have noted.

gpm_api/tests/io/test_checks.py

ghiggi · 2023-09-06T13:50:15Z

Thanks @evanjt for having done this annoying work. The PR is now merged :)
Cc @regislon

Unit tests for gpm_api.io.checks, run coverage on pytest

e6bb56e

evanjt requested review from sphamba and ghiggi July 19, 2023 17:17

evanjt self-assigned this Jul 19, 2023

ghiggi reviewed Jul 20, 2023

View reviewed changes

gpm_api/tests/io/test_checks.py Outdated Show resolved Hide resolved

ghiggi reviewed Jul 20, 2023

View reviewed changes

evanjt added 2 commits July 20, 2023 11:06

Use black, remove bbox check, move bbox test to geospatial test for f…

5b642a6

…uture use, allow check_time to fix incoming str and np objects into check_date()

Cast np datetime [ns] to [us]

1e79137

sphamba requested changes Jul 21, 2023

View reviewed changes

gpm_api/tests/io/test_checks.py Outdated Show resolved Hide resolved

gpm_api/tests/io/test_checks.py Outdated Show resolved Hide resolved

ghiggi reviewed Jul 21, 2023

View reviewed changes

gpm_api/io/checks.py Outdated Show resolved Hide resolved

evanjt added 2 commits July 24, 2023 11:10

Add io/directories tests, modify replace() in curl path constructor a…

8670b55

…s to not replace host (arthurhouftps.pps.eosdis.nasa.gov -> arthurhouftp.pps.eosdis.nasa.gov)

Mock ftplib, add tests to disk, download, info

fb0d33a

evanjt requested a review from sphamba July 24, 2023 13:36

evanjt and others added 2 commits July 24, 2023 15:47

Modify checks to use os.path.sep to cater for Windows paths

3542f84

Add type hint exception for Python 3.8

e60baec

sphamba requested review from sphamba and removed request for sphamba July 24, 2023 14:55

Apply sed command to conftest.py for 3.8 type hints

4a7e561

sphamba approved these changes Jul 24, 2023

View reviewed changes

evanjt added 2 commits July 25, 2023 11:16

Modify type hints to be compatible with Python 3.8

c36f2fe

Remove type annotation hack for python 3.8

9bac278

ghiggi reviewed Jul 25, 2023

View reviewed changes

gpm_api/tests/io/test_directories.py Outdated Show resolved Hide resolved

evanjt added 2 commits August 9, 2023 07:10

test _download_files

4cdf6f3

Add test for file completeness

0700e40

evanjt and others added 3 commits August 16, 2023 09:26

Fix windows paths

eca5f20

Replace : character to - for windows path compatibility

49b55c7

Merge branch 'ghiggi:main' into add_io_tests

2549c72

evanjt marked this pull request as ready for review August 18, 2023 00:48

evanjt requested a review from sphamba August 18, 2023 00:50

sphamba approved these changes Aug 18, 2023

View reviewed changes

gpm_api/tests/io/test_filter.py Outdated Show resolved Hide resolved

evanjt added 3 commits August 21, 2023 11:00

Add tests to validate future date errors if using None end_time in fi…

54c1e81

…lter_by_time(), modify the default None behaviour to choose utcnow() instead of now() to avoid timezone related issues

Update comment, removing TODO

c0358e5

Update docstring explaining defaults in filter_by_time

21375e7

ghiggi reviewed Aug 25, 2023

View reviewed changes

ghiggi reviewed Aug 29, 2023

View reviewed changes

evanjt added 2 commits August 31, 2023 15:41

Remove redundant auth fixtures from conftest, replace with values in …

8827b5e

…the mock_configuration fixture

Replace ns with seconds

106d18e

Update gpm_api/tests/io/test_download.py

36dacd2

Repository owner deleted a comment from evanjt Sep 5, 2023

ghiggi reviewed Sep 5, 2023

View reviewed changes

gpm_api/tests/io/test_checks.py Outdated Show resolved Hide resolved

evanjt added 5 commits September 5, 2023 17:43

Revert to seconds

3921c02

Check timezone, throw error if given and is not UTC

733b76d

When timezone given and UTC, strip tz information. Test case of using…

8cb2d96

… UTC and without

Add more flatten list cases, add condition to download.flatten_list()…

a23b9e1

… as to not flatten an already flattened list

Replace props with info_dict

732b8ed

ghiggi reviewed Sep 6, 2023

View reviewed changes

gpm_api/tests/io/test_checks.py Show resolved Hide resolved

Update gpm_api/tests/io/test_checks.py

03d7578

ghiggi merged commit e39dc20 into ghiggi:main Sep 6, 2023
7 of 9 checks passed

ghiggi deleted the add_io_tests branch September 6, 2023 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit tests for gpm_api.io #8

Add unit tests for gpm_api.io #8

evanjt commented Jul 19, 2023

ghiggi left a comment

sphamba left a comment

codecov bot commented Jul 24, 2023 •

edited

Loading

evanjt commented Jul 24, 2023 •

edited

Loading

sphamba commented Jul 24, 2023

ghiggi commented Jul 24, 2023

sphamba commented Jul 25, 2023

evanjt commented Jul 25, 2023

evanjt commented Aug 18, 2023

ghiggi left a comment

ghiggi left a comment

ghiggi Aug 29, 2023

evanjt Sep 6, 2023

ghiggi Sep 6, 2023

regislon commented Sep 5, 2023

ghiggi commented Sep 5, 2023

evanjt commented Sep 6, 2023

ghiggi commented Sep 6, 2023 •

edited

Loading

Add unit tests for gpm_api.io #8

Add unit tests for gpm_api.io #8

Conversation

evanjt commented Jul 19, 2023

Prework

Related GitHub issues and pull requests

Summary

ghiggi left a comment

Choose a reason for hiding this comment

sphamba left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 24, 2023 • edited Loading

Codecov Report

evanjt commented Jul 24, 2023 • edited Loading

sphamba commented Jul 24, 2023

ghiggi commented Jul 24, 2023

sphamba commented Jul 25, 2023

evanjt commented Jul 25, 2023

evanjt commented Aug 18, 2023

ghiggi left a comment

Choose a reason for hiding this comment

ghiggi left a comment

Choose a reason for hiding this comment

ghiggi Aug 29, 2023

Choose a reason for hiding this comment

evanjt Sep 6, 2023

Choose a reason for hiding this comment

ghiggi Sep 6, 2023

Choose a reason for hiding this comment

regislon commented Sep 5, 2023

ghiggi commented Sep 5, 2023

evanjt commented Sep 6, 2023

ghiggi commented Sep 6, 2023 • edited Loading

codecov bot commented Jul 24, 2023 •

edited

Loading

evanjt commented Jul 24, 2023 •

edited

Loading

ghiggi commented Sep 6, 2023 •

edited

Loading