search

abhinavk99 / jikanpy

Python wrapper for the Jikan API
MIT License
217 stars 29 forks source link

Switch to single HTTP library. #109

Open baseplate-admin opened 1 year ago

baseplate-admin commented 1 year ago

Hi, Thanks for creating this library. Is there any reason that you are including both aiohttp and requests.

Chris-Peterson444 commented 1 year ago

Those were likely just the go-to libraries of the author(s) at the time. Right now, requests gets the job done for the sync code and aiohttp does the same for the async code. Is there functionality that httpx provides the other two don't?

baseplate-admin commented 1 year ago

Hi thanks for responding,

httpx interfaces with both sync and async codes. Instead of doing async for aiohttp and sync for requests, we can support httpx.

Reasoning :

Chris-Peterson444 commented 1 year ago

Addressing your points:

It will cut down development time ( due to supporting one library ).

I'm not sure how true this is, since we will still have to maintain separate code for synchronous and asynchronous parts of the wrapper regardless. It will probably help some get started, but this is heavily dependent on an individual's familiarity with requests and aiohttp.

httpx is slowly becoming the de-facto async requests library.

I am all for using more popular frameworks/libraries if it means more people will be comfortable using and contributing to the project, but frankly, I'm not sure how true this statement is either. According to GitHub's dependency tracker, httpx is used by 91k repos and 3k packages and aiohttp is used by 336k repos and 8k packages. By this metric, aiohttp is more popular (around 3x). Although, aiohttp I believe has also been around longer, so this probably isn't the best metric. I otherwise don't know a good way to quantify popularity (maybe PyPI stats if you can filter out mirrors?), so I would love to hear other reasons for this claim.

Furthermore, even if httpx is - or becomes - the de-facto asynchronous requests library, requests is certainly the de-facto synchronous requests library. Is moving away from aiohttp worth also giving up requests?

We dont have to keep two libraries. If i wanna go sync route jikanpy still installs aiohttp. ( vice versa for async route )

aiohttp is not a lightweight package ( it includes an http server with the client, which just adds bloat )

You make a good point. As a quick and dirty test, if I make virtual environments (Python3.11, Ubuntu) only installing (1) requests, (2) aiohttp, (3) requests + aiohttp, and (4) httpx, and measure the size of total installed packages (trying to account for bloat from dependencies):

  1. 28 MiB- requests only
  2. 34 MiB - aiohttp only
  3. 36 MiB - requests + aiohttp
  4. 29 MiB - httpx only

We see: aiohttp requires an extra 6 MiB if you only needed requests and requests requires an extra 2 MiB if you only needed aiohttp; whereas having both requires an extra 7 MiB compared to just using httpx.

Similarly, a new virtual environment with just a fresh install of jikanpy-v4 takes up 37 MiB. So by this (very liberal) estimate, we could reduce our package footprint up to ~19%.

Lastly, we should compare the performance of httpx with requests and aiohttp. Suppose switching to a unified library makes writing code easier (e.g., minimizing differences between the sync and async parts), we should still prioritize performance regardless. I have never used httpx before so I'll need to look into this.

In summary:

If we can get better performance and reduce the size of our dependencies, I can see that as a compelling reason to switch.

Chris-Peterson444 commented 1 year ago

There are some interesting speed comparisons between aiohttp and httpx here: https://github.com/encode/httpx/issues/838

and it looks like aiohttp is still a winner, but we'll have to run our own tests.