multi erddap search added

[callumrollo]

First attempt at resolving #190

[callumrollo]

ERDDAP.search_all_servers searches multiple servers for a user-supplied string. It returns a dataframe the dataset name, ID, institution and server url. All a user needs to make data queries against those datasets. The user can supply a list of server urls, or it tries all the servers in erddapy.servers

[abkfenris]

Could protocol also be a kwarg? Or at least in the doc string? Right now you have to read the code to figure out that the function only searches for tabledap.

[callumrollo]

That's a good point. Protocol added as kwarg in 4ab1e5a

[callumrollo]

@abkfenris thank you for the review. I hope the subsequent commits have improved this PR. @ocefpaf could you look over this? I'm not sure why the pre-commit is failing again, it worked fine locally :/

[abkfenris]

Nice! It looks a lot cleaner now.

I think there are still some ways that it could be cleaned up even more that could help the test-ability and reuse-ability. Extracting the data transformation and url generation into their own functions would help that.

For example, the current fetch_results() could become two functions that should be come much easier to test individually.

def parse_results(data: bytes) -> Dict[str, DataFrame]:
    df = pd.read_csv(data)
    try:
        df.dropna(subset=[protocol], inplace=True)
    except KeyError:
        return None
    df["Server url"] = url.split("search")[0]
    return {key: df[["Title", "Institution", "Dataset ID", "Server url"]]}


def fetch_results(url: str, key: str, protocol="tabledap") -> Optional[Dict[str, DataFrame]]:
    """
    Parse search results from multiple servers
    """
    data = multi_urlopen(url)
    if data is None:
        return None
    return parse_results(data)

Then parse_results() can be tested by passing it concrete data, and fetch_results() can be tested with parse_results() mocked out in addition to using library like VCR.

parse_results() could also be then used by a server specific search method on erddapy.ERDDAP, or if I wanted to use httpx to make async requests instead. Similarly generating search URL could have multiple uses too.

It might make sense to pull these functions out into a separate file to keep things more organized.

[abkfenris]

I'd suggest splitting out the parsing of data from your data fetching function.

[callumrollo]

Thanks @abkfenris, I've refactored into several smaller functions and pulled all of these out into a separate file. I'll start working on some tests, which should be easier with the more atomic structure you recommended

[callumrollo]

Am I on the right track with tests here? I'm not sure how to proceed with the more involved ones like parse_results how would we mock data for this?

[ocefpaf]

Let's "fold" this one into a the canonical urlopen by making the latter a thin wrapper to this one. That will allow us to cache the results in that one.

Let's tackle this in another PR.

[callumrollo]

Thanks for the help @abkfenris and @ocefpaf! Can we close #32 now?