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

Check query before fetching data from server #12

Open
fitoprincipe opened this issue Dec 22, 2023 · 8 comments · May be fixed by #47
Open

Check query before fetching data from server #12

fitoprincipe opened this issue Dec 22, 2023 · 8 comments · May be fixed by #47

Comments

@fitoprincipe
Copy link
Member

Problem
Commercial user of GEE API get charged for its usage, so every time a test is triggered, it fetches data from server creating cost for the company. Also, it takes time to fetch data, so when trying to run a complete set of tests, it could take long to finish.

Solution
Save the query string into a file (write/read functionality can be found in geetools) and, when the test is triggered, first check if the query string is the same, if True then skip it, if False then fetch.

Context
The version of GEE python API could be something to consider, since the structure of the query could change from version to version, thus a version check could be implemented. The version should be stored somewhere alongside the query string, so to avoid having to modify the string and for example create a new string/dict with the version code, my proposal is to write it as part of the filename, something like: test_some_method_0-1-384.gee

Probably @tylere could have an opinion on this.

@tylere
Copy link

tylere commented Jan 3, 2024

Assuming that the purpose of pytest-gee is to test packages that use the Earth Engine API (either via the Earth Engine Python client library or REST API), you may want to mock responses of the Earth Engine API so the test run without accessing Earth Engine.
https://pytest-mock.readthedocs.io/en/latest/
https://changhsinlee.com/pytest-mock/

@12rambau
Copy link
Member

12rambau commented Feb 5, 2024

Sorry for not answering earlier but the objective is actually to check the Python API response. The problem we've been facing in the past is small changes made by the GEE behavior without notifying developers. The purpose of the lib here is to really asset the result of the server-side computation.

To provide some context, this plugn is a byproduct of the geetools package that create and manipulate server side object. I really need the response to be evaluate to make sur I'm doing the right thing. I actually got an idea from a colleague on a private repo, I'll try to implement it when I have the time (you'll see it's fancy 😄)

@tylere
Copy link

tylere commented Feb 5, 2024

In this case I would suggest setting up two sets of tests:

  1. A set of fast, inexpensive tests that are run whenever new changes are made to pytest-gee. These tests should use mock data.
  2. A set of slow and/or expensive tests that are run infrequently (for example: daily, weekly, or whenever a new release of pytest-gee is made. These tests would access Earth Engine to detect if the GEE server responses have changed.

@12rambau
Copy link
Member

12rambau commented Feb 6, 2024

ah sure this will be done in downstream packages (geetools, ipygee, pygaul... maybe more ?)

@naschmitz
Copy link

naschmitz commented Dec 27, 2024

Just offering my opinion from the EE perspective (cc @schwehr).

The onus should not be on EE users and packages to verify the consistency of Python API calls to EE. I can only speak from the Python team's perspective, but we take backward compatibility and API stability seriously. Here's what we've done in the last year to improve the reliability and consistency of the Python client libs.

  • Individual Python client library changes synced to GitHub. The prior behavior was batching all the changes for a release into a single commit, but often small changes or the intent of the commits are obfuscated away. This change is to prepare for accepting third party contributions to the client libs (no timeline yet) and to do client library development in a more transparent way.
  • Statically defined Python API. The prior behavior was dynamically populating the ee namespace based on a list of algorithms fetched from the server. This change enabled better static analysis/IDE integrations, change tracking of the API surface, and the ability to add unit testing.
  • Increased unit test coverage. Improved the unit test coverage of the Python client libs. These unit tests compare the EE expressions produced by the client against a set of goldens.
  • Internal integration tests. We have a set of integration tests we run on internal Google infrastructure. These tests make calls to a live EE server and validate the outputs. We'd like to run these tests on GitHub infrastructure, but they're time consuming and expensive. This sounds like part of what you're trying to solve here. I'll advocate for these API integration tests being run externally.

With that being said, there is still room for improvement––code changes can still happen that modify the output of an algorithm call.

I like your idea to record server responses. We call these types of tests hermetic tests internally. We record live backend responses, cache them, and replay them in a hermetic test environment. They are a powerful tool for reducing test instability, but are often brittle and expensive to maintain the recordings.

@12rambau
Copy link
Member

12rambau commented Dec 28, 2024

thanks @naschmitz for your detailed answer, I think a simple use case would be the following:

I create a function that removes the system properties from the property names of an object:

def removeSystem(obj) -> ee.list:
    names = obj.propertyNames()
    systemNames = names.filter(ee.Filter.stringStartsWith("item", "system:"))
    return name.removeAll(systemNames)

Which would work. If I understand what you suggest, I should not test consistency of the API call response but simply the API call itself:

def test_remove_system(data_regression, obj):
    list= removeSystem(obj)
    data = ee.serializer.encode(list)
    data_regression.check(data)

Which work consistently as I'm always doing the same API call. But at some point I guess a clever contributor will notify me and say: "hey you can do it better":

def removeSystem(obj) -> ee.list:
    names = obj.propertyNames()
    return names.filter(ee.Filter.stringStartsWith("item", "system:").Not())

And then without testing the actual backend response I cannot check if the new implemented call is still providing the same results.

If I understood Rodrigo's suggestion correctly, it's:
step 1: check if I'm still doing the same API call (then I don't actually request the answer)
step 2: if the API call has changed I check the backend answer

@naschmitz is using ee.serialize the righ way to check the API calls or will they be different upon every call ?

@fitoprincipe
Copy link
Member Author

Thank you @naschmitz for taking the time to reply on this thread 🙂

@12rambau understood my suggestion correctly. It's based on the hypothesis that "If the query string (from ee.serialize) did not change, then the server's response will be the same and therefore so will the result".

Using a simple example:

def my_func() -> ee.Number:
    """A function that returns 4"""
    return ee.Number(2).add(2)

# a normal test
def test_my_func():
    four = my_func()
    assert four.getInfo() == 4

If the package is being updated frequently and it has many tests like this, we'll be making a lot of call unnecessarily. So, we could do:

def test_my_func():
    # open the recorded query string
    with open('serialized_four.json', 'r') as file:
        expected_serialized = json.load(file)  # {"result": "0", "values": {"0": {"constantValue": 4}}}
    
    # get query string of "my_func"
    serialized = my_func().serialize()
    
    # if query string has not changed, test passes
    if serialized == expected_serialized:
        assert True
    
    # if query string has changed, check server's response
    else:
        is_same = four.getInfo() == 4
        assert is_same
        
        # if the server's response is the same, record the new query string
        if is_same:
            # store new expected_serialized
            with open('serialized_four.json', 'w') as file:
                json.dump(serialized, file)

@schwehr
Copy link

schwehr commented Dec 30, 2024

+1 to what @naschmitz said.

The results from .serialize() is the gold standard of knowing that code is requesting the same thing from the EE servers. Caching the result and doing a replay as Nate describes is definitely a good technique. Note that it is important to craft test cases so that the cached result is reasonably small.

An example of the EE api unit tests checking the complete expression graph for an API call: DictionaryTest.test_combine()

Please note that the EE server responses can change over time independent of the client version (for the same client request). Pinning on a client version will not help with that issue. Migrating our internal integration tests to GitHub will help make that more observable.

@12rambau 12rambau linked a pull request Jan 6, 2025 that will close this issue
4 tasks
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 a pull request may close this issue.

5 participants