iobis / pyobis

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

[update] Making UI simpler, and easier for `occurrences` and `checklist` modules #113

Closed ayushanand18 closed 1 year ago

ayushanand18 commented 1 year ago

Overview

This PR aims to bring in a major refactoring to reduce user complexities, and make it much more simple and easy for non-technical users to get adapted to the package easily.

Changes introduced

The flow of data is much more intuitive now. To fetch an occurrence record, one needs to simply execute this:

from pyobis import occurrences
# build a query
query = occurrences.search(scientificname="Mola mola", size=10)
# execute a query
query.execute()
# check obis api url
query.api_url
# check mapper url (if it exists)
query.mapper_url
# get the data
query.data
# or one can easily get the data only if he/she is not interested in API URL, Mapper URL and other properties
data = occurrences.search(scientificname="Mola mola", size=10).execute()

Other changes/improvements

Resolves #111 by introducing automatic detection of best dtypes for dataframe columns using pandas.DataFrame.infer_objects().

Thanks!

7yl4r commented 1 year ago

I think there is an opportunity to reduce code duplication here. ChecklistResponse and OccResponse both have a lot of common code. The commonalities could be put into an abstract parent class. This PR is good as-is, but this is one possible improvement if you are feeling ambitious.

ayushanand18 commented 1 year ago

I think there is an opportunity to reduce code duplication here. ChecklistResponse and OccResponse both have a lot of common code. The commonalities could be put into an abstract parent class. This PR is good as-is, but this is one possible improvement if you are feeling ambitious.

Thank you so much for the review! I agree upon your feedback that there are some functions that are common between them. And we can have a parent class with shared methods.

So, I tried creating an OBISResponse parent class in the utils and implemented inheritance in both the modules. However, I found that I had to eventually initialise objects in the child class as well, and the complexity introduced made it difficult for me to maintain things in future. Also, I extended this implementation into other modules but found that there is nothing common between the 5 modules except some of the initializations which we had to anyway do for child classes as well. Here's the draft for other modules: taxa, nodes, and dataset.

7yl4r commented 1 year ago

I found that I had to eventually initialise objects in the child class as well,

Was this something that couldn't be resolved by using super()?

I find myself rarely implementing OOP in practice, so I am just curious.