Pre commit fix #115
Conversation
…es that are mostly results
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on refactoring and cleaning up the codebase, including improvements to pre-commit configurations, memory management, and logging. My feedback highlights opportunities to reduce duplication in configuration files, improve robustness in environment variable handling, and optimize performance in loops.
| exclude: \.(pdf|svg|png|jpg|jpeg|zip|json)$|^evaluation/(?!.*\.(md|yaml|py)$).*$ | ||
| - id: trailing-whitespace | ||
| - id: pretty-format-json | ||
| args: | ||
| - --autofix | ||
| - --indent=4 | ||
| - --no-sort-keys | ||
| exclude: \.(pdf|svg|png|jpg|jpeg|zip|json)$|^evaluation/(?!.*\.(md|yaml|py)$).*$ |
There was a problem hiding this comment.
The exclude regex patterns are repeated for both end-of-file-fixer and trailing-whitespace. Consider defining a shared anchor or using a more centralized configuration approach to avoid duplication and improve maintainability.
| os.getenv("LOCAL_CONCEPT_MAPPING_API_KEY", "").strip() | ||
| or os.getenv("OPENROUTER_API_KEY", "").strip() | ||
| ) | ||
| v = os.getenv("LOCAL_CONCEPT_MAPPING_API_KEY", "").strip() or os.getenv("OPENROUTER_API_KEY", "").strip() |
There was a problem hiding this comment.
The use of or for environment variable fallback is correct, but ensure that os.getenv is not returning an empty string that might be treated as a valid API key. Using or on a stripped string is generally safe, but explicit checks for non-empty values are more robust.
| t for t in key_terms | ||
| if isinstance(t, str) and t.strip() and t.strip().lower() in text_lower | ||
| ] | ||
| key_terms_valid = [t for t in key_terms if isinstance(t, str) and t.strip() and t.strip().lower() in text_lower] |
There was a problem hiding this comment.
The inject_alignment_concept_mapping_into_ner_entities function is called inside a loop. If this function performs heavy operations, consider batching or optimizing the call to avoid performance bottlenecks.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
=======================================
Coverage ? 15.48%
=======================================
Files ? 28
Lines ? 6850
Branches ? 0
=======================================
Hits ? 1061
Misses ? 5789
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I was getting error since the new changes to |
I run
pre-commit, some issues were fixed automatically, spelling issues and poetry I had to solve manually. I also modify pre-commit to remove the fix that perhaps shouldn't be run, mostly:jsonfiles since I believe these files are always files with results, and shouldn' t be probably formatted, we can think about it laterevaluationdirectory, except foryaml,mdandpyfilesThere are no other changes in this commit. I will add the description to
CONTRIBUTINGin a separate PR, to keep this PR clean.If you want to change specific settings, let me know, otherwise we can just merge it and start using.