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

Add get_open_and_resolved_markets function #57

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Add get_open_and_resolved_markets function #57

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 37 additions & 7 deletions prediction_market_agent_tooling/monitor/monitor_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import pytz
import streamlit as st

from prediction_market_agent_tooling.benchmark.utils import get_manifold_markets_dated
from prediction_market_agent_tooling.benchmark.utils import (
Market,
get_manifold_markets_dated,
)
from prediction_market_agent_tooling.markets.markets import MarketType
from prediction_market_agent_tooling.monitor.markets.manifold import (
DeployedManifoldAgent,
Expand Down Expand Up @@ -34,6 +37,36 @@ def get_deployed_agents(
raise ValueError(f"Unknown market type: {market_type}")


def get_open_and_resolved_markets(
start_time: datetime,
market_type: MarketType,
) -> tuple[list[Market], list[Market]]:
open_markets: list[Market]
resolved_markets: list[Market]

if market_type == MarketType.MANIFOLD:
open_markets = get_manifold_markets_dated(
oldest_date=start_time, filter_="open"
)
resolved_markets = [
m
for m in get_manifold_markets_dated(
oldest_date=start_time, filter_="resolved"
)
if not m.has_unsuccessful_resolution
]

elif market_type == MarketType.OMEN:
# TODO: Add Omen market support: https://github.com/gnosis/prediction-market-agent-tooling/issues/56
open_markets = []
resolved_markets = []

else:
raise ValueError(f"Unknown market type: {market_type}")

return open_markets, resolved_markets
Comment on lines +40 to +67
Copy link

Choose a reason for hiding this comment

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

The get_open_and_resolved_markets function introduces a structured approach to retrieving open and resolved markets based on the market type. A few points to consider:

  1. Correctness and Logic: The function correctly branches based on market_type to handle different market types. However, it currently only supports MarketType.MANIFOLD with a placeholder for MarketType.OMEN. This is a good start, but the TODO comment indicates that support for Omen markets is pending. It's important to track this as a follow-up task to ensure feature completeness.

  2. Performance: By separating the retrieval of open and resolved markets and filtering out markets with unsuccessful resolutions, the function potentially optimizes data processing. However, the performance impact largely depends on the implementation details of get_manifold_markets_dated and the volume of market data.

  3. Error Handling: The function raises a ValueError for unknown market types, which is a good practice for handling unexpected inputs. However, consider logging this error for easier debugging and monitoring.

  4. Maintainability: The function is well-structured and uses clear variable names, making it maintainable. The separation of concerns (retrieving open vs. resolved markets) is well executed.

  5. TODO Comment: The placeholder for Omen market support is acknowledged with a TODO comment. It's crucial to ensure this task is tracked in the project's issue tracker to not lose sight of this pending enhancement.

Overall, the function is a significant improvement in structuring market data retrieval and lays a good foundation for future enhancements.

Consider adding error logging for the ValueError raised for unknown market types to improve debuggability.

Would you like me to open a GitHub issue to track the implementation of Omen market support as indicated by the TODO comment?



def monitor_app() -> None:
settings = MonitorSettings()
start_time = datetime.combine(
Expand All @@ -52,12 +85,9 @@ def monitor_app() -> None:
)

st.subheader("Market resolution")
open_markets = get_manifold_markets_dated(oldest_date=start_time, filter_="open")
resolved_markets = [
m
for m in get_manifold_markets_dated(oldest_date=start_time, filter_="resolved")
if not m.has_unsuccessful_resolution
]
open_markets, resolved_markets = get_open_and_resolved_markets(
start_time, market_type
)
monitor_market(open_markets=open_markets, resolved_markets=resolved_markets)

with st.spinner("Loading agents..."):
Expand Down
Loading