refactor: polymarket

[nikbpetrov]

• please note the dates on the fetch() interface -- i preserved the two separations you have today/fetch_datetime and exposed them via fetch() similar to the other sources, but it seems like an overkill, let me know how you prefer to handle this (or just remove one of them if you prefer to avoid the back-and-forth)

Tests: expected parity between old/new for fetch job, and perfect parity between prod/old/new for update; timing within 10% threshold

[houtanb]

@nikbpetrov I think remove both of them from the argument list. I'm not sure when/why we'd pass either of them to fetch()

today is a date, fetch_datetime is a datetime.

Please rebase this branch on top of origin/main there's a small conflict with the manifold branch. Also please squash all commits and force push them so that there's just one commit above origin/main

[houtanb]

Also @nikbpetrov I don't see why we have this in src/sources/infer.py:

    def fetch(
        self,
        *,
        dfq: DataFrame[QuestionFrame] | None = None,
        files_in_storage: list[str] | None = None,
    ) -> DataFrame[InferFetchFrame]:

and this in src/sources/polymarket.py:

    def fetch(
        self,
        *,
        dfq: DataFrame[QuestionFrame] | None = None,
        existing_resolution_files: dict[str, pd.DataFrame] | None = None,
        today: date | None = None,
        fetch_datetime: str | None = None,
    ) -> DataFrame[PolymarketFetchFrame]:

It seems like you're using flies_in_storage and existing_resolution_files to represent the same concept, which is confusing and should be fixed.

[nikbpetrov]

you're using flies_in_storage and existing_resolution_files to represent the same concept. I want to make sure that after you run AI code reviewers on the codebase, YOU are reviewing all of this code by hand yourself. Can you confirm you are doing this?

I am. I am checking every line by hand. Check comments for instance: AI has so far refused to copy over the same comments you've left through the old code and I have been doing that by hand as I am going through.

I also noticed that difference and noted it but forgot to come back to it...

[houtanb]

@nikbpetrov I think remove both of them from the argument list. I'm not sure when/why we'd pass either of them to fetch()

today is a date, fetch_datetime is a datetime.

Please rebase this branch on top of origin/main there's a small conflict with the manifold branch. Also please squash all commits and force push them so that there's just one commit above origin/main

same for metaculus, it's not clear why today is an argument to fetch()

[houtanb]

I am. I am checking every line by hand. Check comments for instance: AI has so far refused to copy over the same comments you've left through the old code and I have been doing that by hand as I am going through.

I also noticed that difference and noted it but forgot to come back to it...

Oh, no worries then! Thanks so much for all of the thought and effort you put into this!

[nikbpetrov]

same for metaculus, it's not clear why today is an argument to fetch()

As per this comment in the manifold PR.

[nikbpetrov]

Also @nikbpetrov I don't see why we have this in src/sources/infer.py:

    def fetch(
        self,
        *,
        dfq: DataFrame[QuestionFrame] | None = None,
        files_in_storage: list[str] | None = None,
    ) -> DataFrame[InferFetchFrame]:

and this in src/sources/polymarket.py:

    def fetch(
        self,
        *,
        dfq: DataFrame[QuestionFrame] | None = None,
        existing_resolution_files: dict[str, pd.DataFrame] | None = None,
        today: date | None = None,
        fetch_datetime: str | None = None,
    ) -> DataFrame[PolymarketFetchFrame]:

It seems like you're using flies_in_storage and existing_resolution_files to represent the same concept, which is confusing and should be fixed.

I would prefer to change it to existing_resolution_files on infer as a separate PR - is that okay with you?

[houtanb]

I would prefer to change it to existing_resolution_files on infer as a separate PR - is that okay with you?

Yes, I agree, that name is better. Please open a separate PR for that

[nikbpetrov]

fixed fetch() interface and also: note that i've opted to keep them on top of fetch, even if not very readable, so as to have them fixed on the moment the fetch starts (rather than placing them nearer where the code actually runs)

[houtanb]

fixed fetch() interface and also: note that i've opted to keep them on top of fetch, even if not very readable, so as to have them fixed on the moment the fetch starts (rather than placing them nearer where the code actually runs)

Awesome, thanks! Can you also rebase on top of origin/main and resolve the conflict? Please also squash all changes into one commit.

[nikbpetrov]

flagging the .gcloudignore handling in the Makefiles does not work as intended but will do that separately for all refactored Makefiles