baronet2 / FirstCyclingAPI

An unofficial Python API wrapper for firstcycling.com
https://firstcyclingapi.readthedocs.io/en/latest/
MIT License
25 stars 3 forks source link

Adaption for new structure #24

Closed psemdel closed 1 year ago

psemdel commented 1 year ago

Please check it.

psemdel commented 1 year ago

I understand what happens, they changed their system completely with the time. For instance, I changed nothing by rider, but the test fails also. I stay on the topic.

psemdel commented 1 year ago

The different cases are really difficult to cover, but I found after much time a solution that seems to work for many cases (see tests). Don't hesitation to improve the code. I suppose, it is not my last commit.

baronet2 commented 1 year ago

Hi @psemdel , I opened #25 which I think fixes most of the problems with a few changes. I'll wait a few days before merging in case you have any comments.

My PR does not fix the problem with the other classifications for 2023 stage races. If your PR has the solution (I haven't had time to test yet), let's rebase after merging #25. I want to handle one improvement at a time.

psemdel commented 1 year ago

Sure, too many changes at once is dangerous. I had to change several things at the same time, because otherwise it worked for then new race, but not anymore for the old, and then the contrary.. So I tackled everything at once.

Compared to your PR:

My suggestion would be that you first pick-up my supplementary tests. Then you can make up your mind from there.

I also added a handling of startlist and results (I needed it for my purposes). It is an extra feature. I really needed some effort to avoid breaking your code with it. But definitively you can add it at the end. It is the file "combi" and "combi_test". Nothing depends on them.

baronet2 commented 1 year ago

Thanks @psemdel ! I just merged #25 .

I agree with your changes, and like you said let's add them one at a time. Let's do:

  1. Fix the classifications/standings for 2023 stage races, and add the associated tests.
  2. Add handling of startlist endpoints and associated tests.

You can convert this PR to be change 1, or you can make a new PR if that's easier.

psemdel commented 1 year ago

I think, I will make a new PR tomorrow.