chore(ibis): DEBT-1 - drop the last two lazy mountainash imports - #105
Merged
Conversation
_generic_index_exists (_index.py) and IbisBackend.list_indexes (backend.py) each did a call-time 'import mountainash as ma' to read an ibis .sql() result (ma.relation(result).to_dict()/.to_dicts()), the only two remaining sites after PR #93's cutover deleted the module-level dependency. mountainash is not declared in pyproject.toml, so any environment without it installed would crash on first index op. Replaced with native ibis result.to_pyarrow(): - _generic_index_exists: .column(0).to_pylist() - preserves the positional (not alias-name) read Oracle's uppercased COUNT alias requires. - list_indexes: .to_pylist() - same list[dict] shape as ma.relation().to_dicts(). Added regression coverage: _generic_index_exists tested against a mismatched-alias exists_sql_fn (simulates Oracle's uppercasing) to lock in the positional-read contract; list_indexes strengthened to assert exact row-dict keys/values instead of just list length. Verified functionally with the mountainash import blocked at runtime.
|
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
1 participant
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
Closes DEBT-1 - removes the last two lazy
import mountainash as macall sites in the ibis backend (_generic_index_existsin_index.py,IbisBackend.list_indexesinbackend.py). Both were reading an ibis.sql()result viama.relation(result).to_dict()/.to_dicts().mountainashis not declared as a dependency inpyproject.toml, so any environment without it installed would crash the first timeindex_exists/list_indexesran.Fix (Option C, per backlog)
Replaced both with native ibis
result.to_pyarrow():_generic_index_exists:.column(0).to_pylist()- reads the COUNT column by position, preserving the exact behavior thema.relationworkaround existed for (Oracle upper-cases the unquotedcountalias toCOUNT, so keying by name wouldKeyError).list_indexes:.to_pylist()- samelist[dict]row shape asma.relation(result).to_dicts().Zero remaining
import mountainashinsrc/.Tests
TestIndexExistsReadsColumnByPosition(test_index_ops.py) - exercises_generic_index_existsagainst a mismatched-aliasexists_sql_fn(simulates Oracle's uppercasing) to lock in the positional-read contract.test_list_indexes(test_backend.py) - now asserts exact row-dict keys/values instead of just list length.mountainashimport blocked at runtime (bothsqliteandduckdbdialects) - confirms the actual DEBT-1 bug premise (crash whenmountainashisn't installed) no longer reproduces.Verification
Generated with assistance from an AI coding agent.