Skip to content

refactor: yfinance#207

Merged
houtanb merged 7 commits into
forecastingresearch:mainfrom
nikbpetrov:yfinance
Jun 15, 2026
Merged

refactor: yfinance#207
houtanb merged 7 commits into
forecastingresearch:mainfrom
nikbpetrov:yfinance

Conversation

@nikbpetrov

@nikbpetrov nikbpetrov commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Found comments to be overly verbose on main so have stripped most. Code is pretty clear.

Date setting was pretty inconsistent - 78e608d is my attempt at this - inconsistent w/ other sources that don't define an extra prop but instead pass args downstream, but that solution is UGLY here.

Comment thread src/sources/yfinance.py
f"Stock tickers not in top 500 but in current stocks: {set_current - set_top_500}"
)

current_time = dates.get_datetime_now()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

note deviation: this used to be inside the loop

@nikbpetrov

Copy link
Copy Markdown
Collaborator Author

Tests pass w/ perfect parity.

@nikbpetrov nikbpetrov marked this pull request as ready for review June 9, 2026 12:29
@nikbpetrov nikbpetrov requested a review from houtanb June 9, 2026 12:29
@nikbpetrov

Copy link
Copy Markdown
Collaborator Author

Rebased on top of #215

@houtanb

houtanb commented Jun 11, 2026

Copy link
Copy Markdown
Member

Resolution fails with:

Traceback (most recent call last):
  File "/app/main.py", line 21, in <module>
    from sources.registry import SOURCES
  File "/app/sources/registry.py", line 18, in <module>
    from .yfinance import YfinanceSource
  File "/app/sources/yfinance.py", line 13, in <module>
    import yfinance as yf
ModuleNotFoundError: No module named 'yfinance'

@houtanb houtanb merged commit 0e5f16f into forecastingresearch:main Jun 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants