iobis / pyobis

OBIS Python client
https://iobis.github.io/pyobis
MIT License
14 stars 10 forks source link

Add `.get_mapper_url()` function? #69

Closed MathewBiddle closed 2 years ago

MathewBiddle commented 2 years ago

Sometimes it's nice to make sure you're getting the response you would expect, directly from the OBIS mapper - https://mapper.obis.org/

I would like to see an additional response where we can simply return the url for the mapper based on our search configuration. You can use the same url configuration we use for the occurrences.search function but with the following structure

'https://mapper.obis.org/?',occurrence_search_config.replace("occurrence?","")

ayushanand18 commented 2 years ago

We might get into trouble while rendering the get_mapper_url() method since not all OBIS endpoints have an equivalent mapper url, and that mapper uses taxonid to fetch records with scientificname not being a criterion. We just get to search by scientificname in the GUI but at the end it renders with taxonid only. So, if a user queries through scientificname in pyobis we might need to first fetch the taxon id from worms or any other source. For Eg. for Mola mola the OBIS url might look like https://api.obis.org/occurrence?scientificname=Mola%20mola and for mapper it would look like https://mapper.obis.org/?taxonid=127405.

MathewBiddle commented 2 years ago

Can we make get_mapper_url() only available when searching using taxonid ?

ayushanand18 commented 2 years ago

That's one way to get past that error, but I suppose we look at an instance where a researcher wants to look at some species. He does the lookup with occurrences.search with scientificname but when runs the get_mapper_url he finds he should look-up with taxonid instead, so he will need to rephrase the query. That will eventually waste some crucial time he could have spent in something else. We might utilize #55 here when we implement it. We will pick the taxonid for the first suggestion.

ayushanand18 commented 2 years ago

It would look something ike this:

    def get_mapper_url(self):
        """
        Get the corresponding API URL for the query.

        Usage::

            from pyobis.occurrences import OBISQueryresult as OQR
            query = OQR()
            data = query.search(scientificname="Mola mola")
            api_url = query.get_mapper_url()
            print(api_url)
        """
        if not self.args["taxonid"] and self.args["scientificname"]:
            self.args["taxonid"] = self.lookup_taxon(self.args["scientificname"])[0]["id"]

        return "https://mapper.obis.org/" + "?" + urlencode({k:v for k, v in self.args.items() if not v == None})

    def lookup_taxon(self, scientificname):
        """
        Lookup for taxon metadata with scientificname

        Usage::

            from pyobis.occurrences import OBISQueryresult as OQR
            query = OQR()
            lookup_data = query.lookup_taxon(scientificname="Mola mola")
            print(lookup_data)
        """
        res = requests.get(f"https://api.obis.org/v3/taxon/complete/{scientificname}")
        return res.json()
ayushanand18 commented 2 years ago

But for this to work we need to make sure that the number of scientificname we enter must be one or none, not more than that otherwise it will show up an unexpected result.

7yl4r commented 2 years ago

not all OBIS endpoints have an equivalent mapper url

This is where object-oriented programming's inheritance comes in. There may be a way to structure parent & children classes such that we don't have this issue. We could have a broad response class OBISQueryResult(Object) without a mapper functionality, then a class OBISQueryResultMapperEnabled(OBISQueryResult) that inherits from the more generic parent.

That may be enough, but as things increase in complexity it sometimes becomes valuable to consider using python's multiple inheritance features aka "component-entity composition" or "mixins". We would create a MapperFunctionality mixin and use it to compose the OBISQueryResultMapperEnabled class.

I am probably over-engineering on this little package though. Whatever implementation is simplest is going to be best here.

ayushanand18 commented 2 years ago

Using inheritance is a good idea but what I can understand is that it can make things complicated from the user's point of view. I have experimented with inheritance (mutiple and multilevel) in classes but haven't worked with "mixins" before. I have a simpler idea, we can pass on a flag at the end of each member method if that methods supports mapper url or not and make get_mapper_url work only when the flag is true otherwise return no equivalent mapper url. But I don't know if that's the best one.

7yl4r commented 2 years ago

We don't need the perfect solution; a good, simple, and working solution will make me happy. Go ahead with whatever seems right to you; we could always refactor in the future.

ayushanand18 commented 2 years ago

We don't need the perfect solution; a good, simple, and working solution will make me happy. Go ahead with whatever seems right to you; we could always refactor in the future.

Thank you so much! I'll make some those changes and open a PR soon.