fix(preprocess): capture shared-prefix ticker lists - #149
Open
bozarnr wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves quantmind.preprocess.news.extract_exchange_ticker_hints to capture additional comma-separated ticker symbols that share the same supported exchange prefix (e.g., (NYSE: EVEX, EVEXW; ...)), and adds deterministic regressions based on the public examples in issue #108.
Changes:
- Extend exchange-qualified ticker hint extraction to include comma-separated list members following a supported exchange prefix.
- Stop the shared-prefix list scan at
;or)to avoid merging separate groups. - Add regression tests covering the five example shapes from #108.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
quantmind/preprocess/news.py |
Adds shared-prefix list-member extraction for exchange-qualified ticker hints. |
tests/preprocess/test_news.py |
Adds regression cases for shared-prefix lists and related multi-exchange examples. |
Suppressed comments (1)
quantmind/preprocess/news.py:452
- The shared-prefix scan currently runs for every exchange match and starts scanning from
match.end(). If the match already consumed a closing), this will scan beyond the parenthetical (e.g. "(NASDAQ: ABC), a leading..."), and the comma after the)can cause unrelated words to be captured as extra tickers. Consider only scanning list members when the match started a parenthetical group and did not already close it.
tail = re.split(r"[;)]", scan_text[match.end() :], maxsplit=1)[0]
for list_match in _EXCHANGE_TICKER_LIST_MEMBER_RE.finditer(tail):
append_hint(
symbol=list_match.group(1).upper(),
exchange=exchange,
raw=f"{raw_exchange}: {list_match.group(1).upper()}",
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+59
to
+62
| _EXCHANGE_TICKER_LIST_MEMBER_RE = re.compile( | ||
| r"\s*,\s*([A-Z][A-Z0-9.-]{0,9})\b", | ||
| re.IGNORECASE, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #108
Testing
python -m pytest tests/preprocess/test_news.py -k exchange_ticker_hints -o addopts=python -m pytest tests/preprocess/test_news.py -o addopts=python -m ruff check quantmind/preprocess/news.py tests/preprocess/test_news.pygit diff --checkNote: running the targeted
pytestcommand with the repository default addopts produced 3 passing selected tests, then failed the global 75% coverage threshold because only this one test slice was executed.