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

Added property getting company executives #989

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

nikolamilosevic86
Copy link

I have added a functionality that reads company executives with their salaries and dates of births if available. The retrieved information comes in dataframe in property executives.

@ProgrammingDao
Copy link

Happy to help with the review if appropriate.

@ValueRaider
Copy link
Collaborator

Hi Nikola. I'm willing to merge but have one request - move the fetching into it's own function e.g. Ticker._get_executives(). Each scrape url should have its own function.

@fredrik-corneliusson
Copy link
Contributor

@ValueRaider if merged please make sure it does not affect performance of other API:s
New features should be carefully added and be of common use as all new features need to be maintained and supported.

In my opinion before adding more features to the already bloated and buggy base.py the code need to restructured to allow for a more plugin oriented API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants