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.

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

Add Recommendations Trend Summary #1754

Merged
merged 1 commit into from
Dec 2, 2023
Merged

Add Recommendations Trend Summary #1754

merged 1 commit into from
Dec 2, 2023

Conversation

bot-unit
Copy link

@bot-unit bot-unit commented Dec 2, 2023

Add fetching of recommendations as requested from discussion

It's my first pull request (usually i don't write code for other projects, just provide help or code samples ). Tell me, if I am doing something wrong.

For future, i have plans to do some code rewriting, remove stale code, add all avalaible data from yahoo. I have my own project, that recieves more data that this lib.

Fixes #1577

@bot-unit bot-unit changed the base branch from main to dev December 2, 2023 11:07
Copy link
Collaborator

@ValueRaider ValueRaider left a comment

Choose a reason for hiding this comment

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

One more thing: restore/create unit test

yfinance/scrapers/quote.py Outdated Show resolved Hide resolved
yfinance/scrapers/quote.py Outdated Show resolved Hide resolved
yfinance/scrapers/quote.py Outdated Show resolved Hide resolved
@bot-unit
Copy link
Author

bot-unit commented Dec 2, 2023

@ValueRaider should I return pandas.Dataframe() ?
At test case it commented out and require dataframe. But I don't known if we really need dataframe for 4 elements list

@ValueRaider
Copy link
Collaborator

ValueRaider commented Dec 2, 2023

If data is tabular with 2+ rows then Pandas is best, easier to access & analyse. But do this in base.py, I want to work towards separating fetch and format.

@bot-unit
Copy link
Author

bot-unit commented Dec 2, 2023

@ValueRaider I can't move creation Dataframe to base, because all other creations are at quote now. We can plan step by step rewrite all code to new version 0.3.0 with code migration and removing old.

@ValueRaider
Copy link
Collaborator

Then for now, format as Pandas in fetch.

@bot-unit
Copy link
Author

bot-unit commented Dec 2, 2023

If data is tabular with 2+ rows then Pandas is best, easier to access & analyse. But do this in base.py, I want to work towards separating fetch and format.

I think, we should separate fetching, parsing and holding. So we will have ticker class as entry point, scrapers for fetching and something in middle for parsing and transforming.

@ValueRaider
Copy link
Collaborator

Final request - squash commits #1084. Keeps history clean.

added valid modules for quote summary request
added _fetch method for fetching quote summary
added fetch recommendationTrend
@ValueRaider
Copy link
Collaborator

ValueRaider commented Dec 2, 2023

You branched off dev, why you merging main? https://github.com/ranaroussi/yfinance/network

@bot-unit
Copy link
Author

bot-unit commented Dec 2, 2023

@ValueRaider fixed. Sorry for my bad hands :(

@ValueRaider ValueRaider merged commit 1d3ef4f into ranaroussi:dev Dec 2, 2023
@ValueRaider ValueRaider mentioned this pull request Jan 6, 2024
@ValueRaider ValueRaider changed the title Feature/recommendations Add Recommendations Trend Summary Jan 6, 2024
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.

3 participants