Skip to content

Added missing imports, cleaned up unused types and fixed indentations#27

Open
mallasiddharthreddy wants to merge 3 commits intodbpedia:mainfrom
mallasiddharthreddy:fix-redis-isolated
Open

Added missing imports, cleaned up unused types and fixed indentations#27
mallasiddharthreddy wants to merge 3 commits intodbpedia:mainfrom
mallasiddharthreddy:fix-redis-isolated

Conversation

@mallasiddharthreddy
Copy link

@mallasiddharthreddy mallasiddharthreddy commented Jan 27, 2026

This PR resolves a NameError that prevented EntityLinking/test_redis.py from executing.

Changes:

  1. Fixed Crash: Added missing import os (required for os.getenv calls).
  2. Cleanup: Removed unused imports (List, Dict) and fixed indentation to improve code compliance.

Verification:
Validated that the script now passes the import stage and executes without raising a NameError.

Summary by CodeRabbit

  • Tests
    • Cleaned up imports and minor formatting in Redis connection tests; adjustments are non-functional and do not change test behavior or public interfaces.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Removed unused typing imports and a minor whitespace/empty-line change in a single test file; no executable logic or public API was altered.

Changes

Cohort / File(s) Summary
Test File Update
GSoC25/EntityLinking/test_redis.py
Removed typing.List/typing.Dict imports and deleted an empty line after a Redis connection error handling block; no behavioral changes to the test logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Added missing imports, cleaned up unused types and fixed indentations' accurately summarizes the main changes made in the pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@GSoC25/EntityLinking/test_redis.py`:
- Around line 20-28: The try/except block in test_redis.py has inconsistent
indentation (extra space before the lines starting at the try and the except)
causing an IndentationError; fix by aligning the block to use the correct
indentation level for the try, its inner statements (the if/else and
prints/raise), and the except clause so that try, if/else, and except are
consistently indented (match surrounding file style), e.g., ensure the lines
containing try:, the if self.redis_forms.ping() and self.redis_redir.ping():
branch, the else: branch and the except Exception as e: line all use the same
consistent indentation level for block structure around self.redis_forms.ping(),
self.redis_redir.ping(), self.redis_forms.dbsize(), self.redis_redir.dbsize()
and the raised ConnectionError.
- Around line 3-5: There is a duplicate import of the os module in
test_redis.py; remove the redundant second import statement so only a single
"import os" remains (leave the existing "import redis" intact) to eliminate the
unnecessary duplicate import.

@mallasiddharthreddy
Copy link
Author

Hi @RicardoUsbeck , I've resolved the merge conflicts with the recently merged PR #29. This PR (#27) fixes the actual runtime crash in test_redis.py caused by missing imports. It’s ready for review.

@mallasiddharthreddy
Copy link
Author

@mommi84 I noticed the recent discussion in PR #32 regarding the Redis-based Entity Linking architecture.

Since this PR (#27) fixes the baseline tests for that exact Redis module, I’ve verified it’s ready to serve as the foundation for the Dockerized linker.

Let me know if you need any other changes to ensure test_redis.py aligns with the new infrastructure plans!

@NakulSingh156
Copy link

@mommi84 I noticed the recent discussion in PR #32 regarding the Redis-based Entity Linking architecture.

Since this PR (#27) fixes the baseline tests for that exact Redis module, I’ve verified it’s ready to serve as the foundation for the Dockerized linker.

Let me know if you need any other changes to ensure test_redis.py aligns with the new infrastructure plans!

Hi @mallasiddharthreddy!

Thanks for catching that missing import os in the baseline tests! Always good to have those NameError bugs cleaned up.

Just a heads-up on how PR #32 integrates: it actually introduces a fully isolated docker-compose environment and a dynamic seed_redis.py script that handles the data state automatically. Because the new Neuro-Symbolic pipeline spins up, seeds, and tears down its own localized Redis container for entity resolution, it doesn't actually rely on the older standalone test scripts to function.

Your cleanups in #27 are definitely helpful for legacy script maintenance, but the new Docker infrastructure is completely self-contained end-to-end! Cheers.

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.

2 participants