tybug / ossapi

The definitive python wrapper for the osu! api.
https://tybug.github.io/ossapi/
GNU Affero General Public License v3.0
80 stars 17 forks source link

high level abstractions / conveniences #34

Closed tybug closed 2 years ago

tybug commented 3 years ago

api v2 is complex enough that it probably makes sense for us to provide some abstractions to help consumers. Things I've thought of (but not attempted implementing, and so might not work as well as I envisioned):

allow User / UserCompact as arguments (done in b068521cd23b6c84d6de445da86c70490c9307c3)

allow User or UserCompact to be passed to methods that normally require a user_id or username. Internally we'd just take user.id and supply that.

PaginatedModel

instead of exposing cursors to the user directly, return models which handle pagination for us:

class PaginatedModel(Model):
    def __init__(self, cursor, *args, **kwargs):
        self.cursor = cursor
        super().__init__(*args, **kwargs)

    def next(self):
        """
        Retrieves and returns the next page of results. Raises `NoPagesLeftException` if 
        there are no more pages to paginate. You can also check ``next_page_exists`` 
        to see if you can paginate. (TODO: better name than ``next_page_exists``)
        """
        ...

    def next_page_exists(self):
        return bool(self.cursor)

will probably need some restructuring to allow models to make requests (should they also store a reference to their calling api object?)

Then users will never need to know about cursors, unless they want to retrieve a specific attribute, which will still be possible through model.cursor.whatever.

Tough question: should cursors be removed as parameters to functions? The argument for yes is that ideally consumers would not be keeping references to raw cursors in case they need to retrieve the next page, but rather keeping a reference to the entire PaginatedModel.

However this isn't ideal if they hold that object for a long time because then they're also holding all of its response content in memory instead of just the cursor, and responses can be fairly big. Perhaps we should still expose the cursor parameter as an advanced option, but discourage its use unless you know what you're doing? I think that's the right path forward.

Expansion of compact models (done in https://github.com/circleguard/ossapi/commit/c32abe4942e55b25cce80a23f3f9f1491b344593)

Some endpoints only return a *Compact model, but you might want the full model. eg api.search only returns UserCompact, not User. We should have something like the following:

class Expandable(ABC):
    @abstractmethod
    def expand(self):
        pass

class UserCompact(Model, Expandable)
class BeatmapCompact(Model, Expandable)
class BeatmapsetCompact(Model, Expandable)

Then:

user = api.search(query="tybug").user.data[0]
# before
print(api.user(user.id).statistics.ranked_score)
# after
print(user.expand().statistics.ranked_score)
tybug commented 3 years ago

implemented compact model expansion in https://github.com/circleguard/ossapi/commit/c32abe4942e55b25cce80a23f3f9f1491b344593, save for beatmapsetcompact, which will be implemented whenever we implement beatmapset lookup (which in turn is likely to be whenever that endpoint becomes documented)

tybug commented 2 years ago

Going to close this for now as the only remaining unimplemented item is paginated models. Implementing that will be a significant amount of work and so I'm not sure it's worth the effort, but can revisit at a later date (and in a new issue) if I do decide to add it.