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

No Api Keys #243

Merged
merged 5 commits into from
Mar 22, 2024
Merged

No Api Keys #243

merged 5 commits into from
Mar 22, 2024

Conversation

SubramanyamChalla24
Copy link
Contributor

@SubramanyamChalla24 SubramanyamChalla24 commented Mar 13, 2024

Pull Request Title

Removed Vector Databases

Related Issue

Description

Type

  • Bug Fix
  • Feature Enhancement
  • Documentation Update
  • Code Refactoring
  • Other (please specify):

Proposed Changes

Screenshots / Code Snippets (if applicable)

How to Test

Checklist

  • The code compiles successfully without any errors or warnings
  • The changes have been tested and verified
  • The documentation has been updated (if applicable)
  • The changes follow the project's coding guidelines and best practices
  • The commit messages are descriptive and follow the project's guidelines
  • All tests (if applicable) pass successfully
  • This pull request has been linked to the related issue (if applicable)

Additional Information

@SubramanyamChalla24 SubramanyamChalla24 changed the title No api keys Removed Vector Databases Mar 13, 2024
@SubramanyamChalla24 SubramanyamChalla24 changed the title Removed Vector Databases No Api Keys Mar 13, 2024
@srbhr
Copy link
Owner

srbhr commented Mar 13, 2024

Thanks a lot, @SubramanyamChalla24, for this PR. It's much needed, as it'll ease the process of knowing the % match score of the resumes and the JDs.

Before merging, I will be testing out this PR on:

  1. Mac OS 14
  2. Unbuntu 22.04 on WSL

Python Version will be 3.11 (the one with GIL)

@srbhr
Copy link
Owner

srbhr commented Mar 13, 2024

@Devasy23 @imhalcyon @Sayvai Please can you help me in testing this on:

  • Windows
  • Docker
  • Different versions of Python 3.10 to 3.12 (I'm doing 3.11)

@srbhr
Copy link
Owner

srbhr commented Mar 18, 2024

@SubramanyamChalla24, the changes are working smoothly on MacOS!

image

I'll keep this PR open if anyone wants to test it on Windows, and Linux. Else, by Friday I will merge it.

@Devasy23
Copy link
Contributor

Devasy23 commented Mar 19, 2024

@Devasy23 @imhalcyon @Sayvai Please can you help me in testing this on:

  • Windows
  • Docker
  • Different versions of Python 3.10 to 3.12 (I'm doing 3.11)

I'll review before Friday, but the dependencies weren't working for me on Linux with python 3.10, but they are working for 3.11.5 specified in doc

@srbhr
Copy link
Owner

srbhr commented Mar 19, 2024

Okay then we make sure that we'll mention Python 3.11 specifically

@srbhr
Copy link
Owner

srbhr commented Mar 19, 2024

Also, we'll have to clean the Requirements.txt file as well

else:
print("Config file does not exist.")

resume_string = ' '.join(selected_file["extracted_keywords"])
Copy link
Contributor

Choose a reason for hiding this comment

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

The refactoring update simplifies variable assignments and updates method access, which is good. A suggestion would include error handling around the new function get_score to ensure robustness.

Example:

try:
    result = get_score(resume_string, jd_string)
    similarity_score = result[0].score
    st.write("Similarity Score obtained for the resume and job description is:", similarity_score)
except Exception as e:
    st.error(f"Failed to get similarity score: {e}")

Copy link
Owner

Choose a reason for hiding this comment

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

@Devasy23, maybe you can have your fix ready when this merges on Friday. And then do a PR to master with the add-ons/enhancements.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but should I push to @SubramanyamChalla24 's PR or create a new PR to patch those changes

Copy link
Contributor

Choose a reason for hiding this comment

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

Your code has a good structure and makes use of proper logging and exception handling. Here are a few suggestions for improvement:

  1. Avoid Repeated Code: The logging configuration is done twice, both at the beginning and when file_handler and console_handler are set up. Consider keeping only one of those configurations or merge them if different configurations are necessary for different handlers.

  2. Exception Handling in read_config: Returning None when encountering an error might lead to type errors later. Consider raising an exception or ensure that the calling code can handle None types.

Here's a small code improvement for logging setup:

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")

console_handler = logging.StreamHandler()
console_handler.setFormatter(formatter)
console_handler.setLevel(logging.DEBUG)

file_handler = logging.FileHandler("app_similarity_score.log")
file_handler.setFormatter(formatter)
file_handler.setLevel(logging.DEBUG)

logger.addHandler(file_handler)
logger.addHandler(console_handler)

Alternatively you could had also reused the logger from logger.py

@srbhr
Copy link
Owner

srbhr commented Mar 21, 2024

@Devasy23, those are some excellent add-ons. Once this PR gets merged tomorrow, you can do a PR that updates this.

@srbhr srbhr merged commit f668ffc into srbhr:main Mar 22, 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