-
Notifications
You must be signed in to change notification settings - Fork 154
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
Add tests to i.modis, i.landsat, i.sentinel download #1033
Comments
Hello, I am looking forward to contributing to "Add EODAG support to GRASS GIS" for GSoC. Is anyone working on this issue? If not, I will be adding tests for them. |
@HamedElgizery The two thumbs most probably mean you're good to go. Otherwise you'd might only get an answer during the weekend. We'll be happy to help you when reviewing ;) |
Nice. See also OSGeo/grass#3322 (comment) regarding download tests. There may also be a basic question of using unit tests or pytest. Beyond GRASS unit tests for other modules, Pytests for e.g. sentinelsat (https://github.com/sentinelsat/sentinelsat/tree/main/tests) may be worth having a look at... |
@ninsbl Also, I have been looking through i.sentinel.download, and I was trying to run it, but it seems like I have to fully compile GRASS to be able to run the addons, is there a way to work with the addons modules without having to compile the GRASS core source code? |
|
According to @marisn comment seems not a good idea to test the download, but how can we check these modules? Another topic is how to manage user and password for the different services, is there a suggested way to do that? |
Tests should not depend on external systems unless you test those external systems. If network connection or remote server goes down, it will cause test failure although the code in question is just fine. I haven't looked into the code, but generally there are two ways of action:
Connecting to external system during a test is fine iff you test if URLs/API of external system haven't changed. Still then failures of tests should not count against our code and should just generate a warning (as the issue might be just of temporary nature). |
In principle, credentials can be stored in GitHub secrets and made available as environment variables in the workflow. However, some systems (like ASF) require users to rotate their password, unfortunately without an option to do so programatically. So that would be a maintenance issue. However, with @marisn `s suggestions connecting to external systems might not be necessary at all.... |
Given @marsin `s useful comments, I would suggest to add a "p-flag" to i.sentinel.download, that returns the actual query string that would be send to the DHUS repository. Here is an example that should work:
This should return:
That way users can inspect a query before it is sent and the result can be used in classic unit tests. Other stuff could be tested in doc ests I guess...? Anyway, for the GSoC skills test, I would suggest writing a set of unit tests for the query compiled by sentinelsat should be sufficient, no? Not sure about the landsatxplore API... P.S.: I am about to send a PR for some minor issues with |
landsatxplore API is no longer maintained by its developer unfortunately, so the use of something else in i.landsat.download is kinda urgent to ensure the extension is functional again. Both Markus and I were granted admin rights in landsatxplore repo, but it's pretty difficult to keep up. |
See: #1044 for |
I peaked into the i.sentinel.download code – it seemed to be well split into functions and thus it should be easy¹ to test those functions separately. Python mock library is great for uncoupling functions from the rest of code. E.g. ¹ for certain meanings of word "easy". |
Since we have limited time (deadline is April 2nd) what about writing tests for any of the imagery modules that are missing them? See for example i.landsat.toar or i.pca. |
Is there a document, or a research paper by chance, that describes how i.pca works? I am also thinking of using a standard input -- maybe the ones used in the HTML document -- and save the inputs and the outputs for each function into the tests for now... |
Is the documentation linked to in https://grass.osgeo.org/grass-devel/manuals/i.pca.html enough? There is a paper, and another article on the wiki https://grasswiki.osgeo.org/wiki/Principal_Components_Analysis, that is quite filled, with even more info. PCA is also common in machine learning/Artificial intelligence when doing some feature engineering, it is one of the old math tricks available. Since it is a hot topic now, you'd be able to find content on this everywhere outside of the GRASS context. |
Sounds good, thanks! |
That too is nice and easy to read to understand the concept https://medium.com/@dareyadewumi650/understanding-the-role-of-eigenvectors-and-eigenvalues-in-pca-dimensionality-reduction-10186dad0c5c It's pure maths (or engineering maths too). |
@HamedElgizery do you need any help? |
Sorry for the late reply in this issue, @lucadelu . I have read about PCA and I will be working on writing an initial test for it. It is still a bit confusing to me how the input is formatted in the i.pca module, though. |
@HamedElgizery remember to introduce yourself to the extended GRASS community via the grass-dev mailing list and share your proposal there as other candidates have done so far, see eg: https://www.mail-archive.com/[email protected]/msg61769.html. |
I have gone through the suggestions and added them accordingly. I also sent an email to introduce myself, and I have shared the draft through the mailing list. 🙏 |
Hi @HamedElgizery and welcome. The simplest way to add a test to
You may modify other aspects of the i.vi-test, like using Good luck and let us know if you have any more specific questions while working on the test... |
@ninsbl Thank you for the guidance it helped me get started! |
Add test to the following modules:
The text was updated successfully, but these errors were encountered: