search

backtrackbaba / cowin

Python wrapper for CoWin API's
MIT License
42 stars 18 forks source link

Vaccine and Dose filters added #25

Closed tshrv closed 3 years ago

tshrv commented 3 years ago

I've added two more filters:

Updated filter function mechanism to accommodate multiple filters with a dry approach. Updated param typo min_age_limt -> min_age_limit

DragonWarrior15 commented 3 years ago

If you could contrast this with PR #15 and PR #17, would be great.

tshrv commented 3 years ago

Sure @DragonWarrior15,

The following are the features present in PR #25 in contrast with PR #15 and PR #17 -

  1. Instead of putting filters in a dictionary, putting them as the arguments will make it more editor friendly and will have less scope of human error.
  2. Filter Vaccine is an Enum. Editor friendly and reduce chances of human error. cowin_api.constants.py
  3. Filter Dose is an Enum, to filter centers for availability(atleast 1) of the first or second dose. cowin_api.constants.py
  4. Since there are limited number of parameters based on which we might need to filter (currently min_age_limit, vaccine, dose), it is better to have a simpler filter mechanism even if it involves explicitly checking for multiple filters. cowin_api.utils.py

Missing features:

  1. Filter Fees
DragonWarrior15 commented 3 years ago

Instead of putting filters in a dictionary, putting them as the arguments will make it more editor friendly and will have less scope of human error.

Not sure how this works. Dictionary is also a well formatted structure in any editor.

Filter Vaccine is an Enum. Editor friendly and reduce chances of human error. cowin_api.constants.py Isn't this tedious than simply passing a string as input ? I understand that you are trying to avoid spelling mistakes here.

Further, you maybe limited to what vaccines are put in the Enum, and the API will keep evolving as more and more vaccines are being brought in.

Since there are limited number of parameters based on which we might need to filter (currently min_age_limit, vaccine, dose), it is better to have a simpler filter mechanism even if it involves explicitly checking for multiple filters. cowin_api.utils.py

You never know how this will keep growing, so your explicit filtering will require multiple modifications to the code as it evolves. I mean both arguments are kind of right here I guess.

Filter Fees

This can be easily added in the dictionary. The core user functions that take arguments are simply there for compatibility with older versions of the API.

tshrv commented 3 years ago

Sure, You're right. It's already great.

DragonWarrior15 commented 3 years ago

Hey, you can keep the PR open, this merged with PR #17 will probably be useful. Not sure why you closed it.