Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updating Metaculus models #483

Merged
merged 3 commits into from
Oct 10, 2024
Merged

Updating Metaculus models #483

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7572fe3
Updating Metaculus models
kongzii Oct 9, 2024
10f11ce
Fix model
kongzii Oct 9, 2024
436c9f0
fix model
kongzii Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 58 additions & 66 deletions prediction_market_agent_tooling/markets/metaculus/data_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,93 +3,85 @@

from pydantic import BaseModel

from prediction_market_agent_tooling.gtypes import Probability
from prediction_market_agent_tooling.tools.utils import DatetimeUTC


class QuestionType(str, Enum):
forecast = "forecast"
notebook = "notebook"
discussion = "discussion"
claim = "claim"
group = "group"
conditional_group = "conditional_group"
multiple_choice = "multiple_choice"
binary = "binary"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider simplifying QuestionType Enum with only one member

The QuestionType enum now contains only a single member binary. Enums are typically used to define a set of constant values. If no other question types are expected, it might be more appropriate to replace the enum with a simple constant or a type alias to reduce unnecessary complexity.

Suggested change:

-from enum import Enum
+from typing import Literal

-class QuestionType(str, Enum):
-    binary = "binary"
+QuestionType = Literal["binary"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
binary = "binary"
from typing import Literal
QuestionType = Literal["binary"]



class CommunityPrediction(BaseModel):
y: list[float]
q1: float | None = None
q2: float | None = None
q3: float | None = None
class AggregationItem(BaseModel):
start_time: DatetimeUTC
end_time: DatetimeUTC | None
forecast_values: list[float] | None
forecaster_count: int
interval_lower_bounds: list[float] | None
centers: list[float] | None
interval_upper_bounds: list[float] | None
means: list[float] | None
histogram: list[float] | None
Comment on lines +14 to +23
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use Optional from typing for optional type hints

In the AggregationItem class, several fields are annotated with the union syntax | None to indicate that they can be None. For better compatibility with earlier Python versions (prior to 3.10) and to improve readability, consider importing Optional from the typing module.

Apply the following changes:

-from typing import Any
+from typing import Any, Optional

 class AggregationItem(BaseModel):
     start_time: DatetimeUTC
-    end_time: DatetimeUTC | None
-    forecast_values: list[float] | None
+    end_time: Optional[DatetimeUTC]
+    forecast_values: Optional[list[float]]
     forecaster_count: int
-    interval_lower_bounds: list[float] | None
-    centers: list[float] | None
-    interval_upper_bounds: list[float] | None
-    means: list[float] | None
-    histogram: list[float] | None
+    interval_lower_bounds: Optional[list[float]]
+    centers: Optional[list[float]]
+    interval_upper_bounds: Optional[list[float]]
+    means: Optional[list[float]]
+    histogram: Optional[list[float]]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
class AggregationItem(BaseModel):
start_time: DatetimeUTC
end_time: DatetimeUTC | None
forecast_values: list[float] | None
forecaster_count: int
interval_lower_bounds: list[float] | None
centers: list[float] | None
interval_upper_bounds: list[float] | None
means: list[float] | None
histogram: list[float] | None
from typing import Any, Optional
class AggregationItem(BaseModel):
start_time: DatetimeUTC
end_time: Optional[DatetimeUTC]
forecast_values: Optional[list[float]]
forecaster_count: int
interval_lower_bounds: Optional[list[float]]
centers: Optional[list[float]]
interval_upper_bounds: Optional[list[float]]
means: Optional[list[float]]
histogram: Optional[list[float]]


@property
def p_yes(self) -> float:
"""
q2 corresponds to the median, or 'second quartile' of the distribution.

If no value is provided (i.e. the question is new and has not been
answered yet), we default to 0.5.
"""
return self.q2 if self.q2 is not None else 0.5

class Aggregation(BaseModel):
history: list[AggregationItem]
latest: AggregationItem | None
score_data: dict[str, Any]

class Prediction(BaseModel):
t: DatetimeUTC
x: float

class Aggregations(BaseModel):
recency_weighted: Aggregation
unweighted: Aggregation
single_aggregation: Aggregation
metaculus_prediction: Aggregation

class UserPredictions(BaseModel):
id: int
predictions: list[Prediction]
points_won: float | None = None
user: int
username: str
question: int


class CommunityPredictionStats(BaseModel):
full: CommunityPrediction
unweighted: CommunityPrediction
class Question(BaseModel):
aggregations: Aggregations
my_forecasts: Aggregation
type: QuestionType
possibilities: dict[str, str] | None


class MetaculusQuestion(BaseModel):
"""
https://www.metaculus.com/api2/schema/redoc/#tag/questions/operation/questions_retrieve
"""

active_state: Any
url: str
page_url: str
id: int
author: int
author_name: str
author_id: int
author_username: str
title: str
title_short: str
group_label: str | None = None
resolution: int | None
resolved_option: int | None
created_time: DatetimeUTC
publish_time: DatetimeUTC | None = None
close_time: DatetimeUTC | None = None
effected_close_time: DatetimeUTC | None
resolve_time: DatetimeUTC | None = None
possibilities: dict[Any, Any] | None = None
scoring: dict[Any, Any] = {}
type: QuestionType | None = None
user_perms: Any
weekly_movement: float | None
weekly_movement_direction: int | None = None
cp_reveal_time: DatetimeUTC | None = None
edited_time: DatetimeUTC
last_activity_time: DatetimeUTC
activity: float
created_at: DatetimeUTC
published_at: DatetimeUTC
scheduled_close_time: DatetimeUTC
scheduled_resolve_time: DatetimeUTC
user_permission: str
comment_count: int
votes: int
community_prediction: CommunityPredictionStats
my_predictions: UserPredictions | None = None
question: Question
# TODO add the rest of the fields https://github.com/gnosis/prediction-market-agent-tooling/issues/301

@property
def page_url(self) -> str:
return f"https://www.metaculus.com/questions/{self.id}/"

@property
def p_yes(self) -> Probability:
if self.question.type != QuestionType.binary:
raise ValueError(f"Only binary markets can have p_yes.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unnecessary f prefix in exception message

At line 67, the ValueError is using an f-string without any placeholders. The f prefix is unnecessary and can be removed to clean up the code.

Apply this change:

-                raise ValueError(f"Only binary markets can have p_yes.")
+                raise ValueError("Only binary markets can have p_yes.")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
raise ValueError(f"Only binary markets can have p_yes.")
raise ValueError("Only binary markets can have p_yes.")
🧰 Tools
🪛 Ruff

67-67: f-string without any placeholders

Remove extraneous f prefix

(F541)

if (
self.question.aggregations.recency_weighted is None
or self.question.aggregations.recency_weighted.latest is None
or self.question.aggregations.recency_weighted.latest.forecast_values
is None
):
# If no value is provided (i.e. the question is new and has not been answered yet), we default to 0.5.
return Probability(0.5)
if len(self.question.aggregations.recency_weighted.latest.forecast_values) != 2:
raise ValueError(
f"Invalid logic, assumed that binary markets will have two forecasts, got: {self.question.aggregations.recency_weighted.latest.forecast_values}"
)
# Experimentally figured out that they store "Yes" at index 1.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱 why so complicated metaculuuuuuus!!

return Probability(
self.question.aggregations.recency_weighted.latest.forecast_values[1]
)
Comment on lines +80 to +98
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Handle non-binary question types gracefully in p_yes property

Currently, the p_yes property raises a ValueError if the question type is not binary. To make the property safer and more user-friendly, consider returning None or a default value when the question type is not binary. This avoids exceptions and allows calling code to handle non-binary questions appropriately.

Suggested refactor:

 def p_yes(self) -> Probability | None:
-    if self.question.type != QuestionType.binary:
-        raise ValueError("Only binary markets can have p_yes.")
+    if self.question.type != QuestionType.binary:
+        return None
     if (
         self.question.aggregations.recency_weighted is None

Additionally, update the return type annotation to reflect that None can be returned:

-    def p_yes(self) -> Probability:
+    def p_yes(self) -> Probability | None:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def p_yes(self) -> Probability:
if self.question.type != QuestionType.binary:
raise ValueError(f"Only binary markets can have p_yes.")
if (
self.question.aggregations.recency_weighted is None
or self.question.aggregations.recency_weighted.latest is None
or self.question.aggregations.recency_weighted.latest.forecast_values
is None
):
# If no value is provided (i.e. the question is new and has not been answered yet), we default to 0.5.
return Probability(0.5)
if len(self.question.aggregations.recency_weighted.latest.forecast_values) != 2:
raise ValueError(
f"Invalid logic, assumed that binary markets will have two forecasts, got: {self.question.aggregations.recency_weighted.latest.forecast_values}"
)
# Experimentally figured out that they store "Yes" at index 1.
return Probability(
self.question.aggregations.recency_weighted.latest.forecast_values[1]
)
def p_yes(self) -> Probability | None:
if self.question.type != QuestionType.binary:
return None
if (
self.question.aggregations.recency_weighted is None
or self.question.aggregations.recency_weighted.latest is None
or self.question.aggregations.recency_weighted.latest.forecast_values
is None
):
# If no value is provided (i.e. the question is new and has not been answered yet), we default to 0.5.
return Probability(0.5)
if len(self.question.aggregations.recency_weighted.latest.forecast_values) != 2:
raise ValueError(
f"Invalid logic, assumed that binary markets will have two forecasts, got: {self.question.aggregations.recency_weighted.latest.forecast_values}"
)
# Experimentally figured out that they store "Yes" at index 1.
return Probability(
self.question.aggregations.recency_weighted.latest.forecast_values[1]
)
🧰 Tools
🪛 Ruff

67-67: f-string without any placeholders

Remove extraneous f prefix

(F541)

Comment on lines +81 to +98
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making the p_yes property more robust to data format changes

In the p_yes property, the index 1 is hardcoded with the assumption that "Yes" is stored at that position in forecast_values. This reliance on a specific index may lead to errors if Metaculus changes their data format in the future. To improve robustness, consider identifying the correct index dynamically or using a key-value mapping if available.

🧰 Tools
🪛 Ruff

82-82: f-string without any placeholders

Remove extraneous f prefix

(F541)



class MetaculusQuestions(BaseModel):
next: str | None
Expand Down
11 changes: 5 additions & 6 deletions prediction_market_agent_tooling/markets/metaculus/metaculus.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,12 @@ def from_data_model(model: MetaculusQuestion) -> "MetaculusAgentMarket":
question=model.title,
outcomes=[],
resolution=None,
current_p_yes=Probability(model.community_prediction.full.p_yes),
created_time=model.created_time,
close_time=model.close_time,
url=model.url,
current_p_yes=model.p_yes,
created_time=model.created_at,
close_time=model.scheduled_close_time,
url=model.page_url,
Comment on lines +40 to +43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

🛠️ Refactor suggestion

Inconsistent attribute usage with data_models.py

The from_data_model method references attributes (p_yes, scheduled_close_time, page_url, my_forecasts) that are not present in the MetaculusQuestion or Question classes within data_models.py. Please verify these attributes are correctly defined or update the method to align with the existing data models.

🔗 Analysis chain

Verify attribute assignments and update documentation

The changes in the from_data_model method reflect updates to the MetaculusQuestion model structure. Please ensure these modifications maintain the intended functionality:

  1. Confirm that model.p_yes is the correct attribute and doesn't require the Probability wrapper.
  2. Verify that created_at, scheduled_close_time, and page_url are the correct attribute names in the updated MetaculusQuestion model.
  3. Check if the new logic for have_predicted using model.question.my_forecasts.latest is accurate.

Consider adding type hints to improve code clarity:

@staticmethod
def from_data_model(model: MetaculusQuestion) -> "MetaculusAgentMarket":
    return MetaculusAgentMarket(
        id=str(model.id),
        question=model.title,
        outcomes=[],
        resolution=None,
        current_p_yes=model.p_yes,  # type: Probability
        created_time=model.created_at,  # type: DatetimeUTC
        close_time=model.scheduled_close_time,  # type: DatetimeUTC
        url=model.page_url,  # type: str
        volume=None,
        have_predicted=model.question.my_forecasts.latest is not None,  # type: bool
        outcome_token_pool=None,
    )

Update the method's docstring to reflect these changes and explain the new attribute mappings.

To ensure consistency with the data_models.py file, run the following script:

Also applies to: 45-45

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the attribute names in the MetaculusQuestion model

# Test: Check for the presence of new attribute names
rg --type python 'class MetaculusQuestion.*?:.*?(p_yes|created_at|scheduled_close_time|page_url)' prediction_market_agent_tooling/markets/metaculus/data_models.py

# Test: Check for the presence of my_forecasts attribute
rg --type python 'class Question.*?:.*?my_forecasts' prediction_market_agent_tooling/markets/metaculus/data_models.py

Length of output: 346

Script:

#!/bin/bash
# Description: Verify the attribute names in the MetaculusQuestion model

# Test: Check for the presence of new attribute names in MetaculusQuestion
rg -g "*.py" 'class\s+MetaculusQuestion.*?\n(?:.*\n)*?.*\b(p_yes|created_at|scheduled_close_time|page_url)\b' prediction_market_agent_tooling/markets/metaculus/data_models.py

# Test: Check for the presence of my_forecasts attribute in Question class
rg -g "*.py" 'class\s+Question.*?\n(?:.*\n)*?.*\bmy_forecasts\b' prediction_market_agent_tooling/markets/metaculus/data_models.py

Length of output: 692

Script:

#!/bin/bash
# Description: Verify the attribute names in the MetaculusQuestion model with multiline support

# Test: Check for the presence of new attribute names in MetaculusQuestion
rg -U -g "*.py" 'class\s+MetaculusQuestion.*?\n(?:.*\n)*?.*\b(p_yes|created_at|scheduled_close_time|page_url)\b' prediction_market_agent_tooling/markets/metaculus/data_models.py

# Test: Check for the presence of my_forecasts attribute in Question class
rg -U -g "*.py" 'class\s+Question.*?\n(?:.*\n)*?.*\bmy_forecasts\b' prediction_market_agent_tooling/markets/metaculus/data_models.py

Length of output: 1245

volume=None,
have_predicted=model.my_predictions is not None
and len(model.my_predictions.predictions) > 0,
have_predicted=model.question.my_forecasts.latest is not None,
outcome_token_pool=None,
)

Expand Down
2 changes: 1 addition & 1 deletion tests/markets/test_markets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_get_pool_tokens(market_type: MarketType) -> None:

@pytest.mark.parametrize("market_type", list(MarketType))
def test_get_markets(market_type: MarketType) -> None:
limit = 10
limit = 100
markets = market_type.market_class.get_binary_markets(
limit=limit, sort_by=SortBy.NONE, filter_by=FilterBy.OPEN
)
Expand Down
Loading