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

fix bs4 and quote issue #94

Merged
merged 11 commits into from
Oct 24, 2023
Merged

fix bs4 and quote issue #94

merged 11 commits into from
Oct 24, 2023

Conversation

lit26
Copy link
Owner

@lit26 lit26 commented Oct 14, 2023

Description

Fix #90 #92

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code styling or code optimize

What you did

@lit26 lit26 mentioned this pull request Oct 14, 2023
5 tasks
@Mytak
Copy link

Mytak commented Oct 14, 2023

The current commit does not provide the company name.

Suggestion:

Add the following code between lines 137 and 139 of finvizfinance/quote.py

    quote_header = self.soup.find("div", class_="quote-header")
    quote_header_left = quote_header.find("div", class_="quote-header_left")
    quote_header_ticker_wrapper = quote_header_left.find("div", class_="quote-header_ticker-wrapper")
    quote_header_ticker_wrapper_company = quote_header_ticker_wrapper.find("h2")
    if quote_header_ticker_wrapper_company.find("a") != None:
        company_name = quote_header_ticker_wrapper_company.find("a")
    else:
        company_name = quote_header_ticker_wrapper_company
    fundament_info["Company"] = str.strip(company_name.text)

quote.zip

@Bozmo
Copy link

Bozmo commented Oct 18, 2023

Updated finvizfinance 0.14.7rc2 AND beatufifulsoup4 4.12.2

Screener works for me now.

@lit26 lit26 merged commit 6771148 into master Oct 24, 2023
@Ponny035
Copy link

Subject: Variable Name Change from fundament_info["Sector"] to fundament_info["Section"] in finvizfinance/quote.py

Review:

Hey there @lit26 ,

I just noticed the change from fundament_info["Sector"] to fundament_info["Section"] in the code. I'm curious about the reason behind this tweak. Is there a specific reason for this change?

  1. Clarity and Consistency:

While "Section" seems to convey a similar idea as "Sector" it might confuse people who are used to the old naming. Can you share why you made the change and whether it enhances clarity?

  1. Comments and Documentation:

Adding a brief note in the code or updating the documentation could be a helpful addition. When there's a public release, it will help everyone understand what's going on and why.

  1. Impact on Other Code:

Perhaps, you could review the codebase to ensure that this variable name change doesn't have unintended consequences or break any existing functionality or code that relies on fundament_info["Sector"].

  1. Naming Conventions:

Does this project follow a specific naming convention or style guide? If so, does this change align with it? Consistency in naming is crucial for maintaining a clean and easy-to-work-on codebase.

  1. Communication:

Sharing your thoughts on changes like these with the other contributors can help prevent confusion and ensure everyone is on the same page.

I totally understand that we all have different preferences when it comes to coding. I'm just looking to understand the reasoning behind the change and make sure it doesn't cause any hiccups. However, discussing these changes openly can help improve the project for everyone.

Looking forward to hearing your thoughts on this! Thanks for your work on the project.

Cheers,
@Ponny035

@lit26
Copy link
Owner Author

lit26 commented Nov 2, 2023

I think this is changed by mistake. Will fix it.

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.

BUG: Screen Does't work !!!!
4 participants