Skip to content

feat: support pylance 4.0.0#6678

Open
XuQianJin-Stars wants to merge 3 commits intoEventual-Inc:mainfrom
XuQianJin-Stars:support-pylance-4.0.0
Open

feat: support pylance 4.0.0#6678
XuQianJin-Stars wants to merge 3 commits intoEventual-Inc:mainfrom
XuQianJin-Stars:support-pylance-4.0.0

Conversation

@XuQianJin-Stars
Copy link
Copy Markdown
Contributor

Summary

Upgrade pylance dependency to support version 4.0.0 (issue #6677):

  • pyproject.toml : update optional dependency from pylance<0.40.0 to pylance>=0.39.0,<5.0.0, and pin dev dependency to pylance==4.0.0
  • lance_scalar_index.py : rename fragment_uuid parameter to index_uuid to adapt pylance 4.0.0 API change
  • lance_scalar_index.py : fix replace logic in create_scalar_index_internal to include old indices in removed_indices when committing, preventing panic in pylance 4.0.0
  • lance_data_sink.py : handle "not a directory" error in _load_existing_dataset, as pylance 4.0.0 changed the error message format when target path is a file
  • test_lancedb_scalar_index.py : update error message match for invalid index type test to include the new RTREE index type added in pylance 4.0.0

All changes are backward compatible with pylance >=0.39.0.

Test plan

  • tests/io/lance/ : 56 passed — core lance read/write/scan operations
  • tests/io/lancedb/test_lancedb_writes.py : all passed — including test_create_mode_fails_when_target_is_file (fixed error message handling)
  • tests/io/lancedb/test_lancedb_scalar_index.py::TestDistributedIndexing : all passed — including test_build_distributed_index_replace_true_overwrite_existing (fixed removed_indices logic) and test_build_distributed_index_invalid_index_type (updated RTREE match)
  • tests/io/lancedb/ : 133 passed, 2 skipped — full lancedb test suite
  • Total: 189 passed, 2 skipped, 0 failed

- Update pylance dependency range to >=0.39.0,<5.0.0
- Pin dev dependency to pylance==4.0.0
- Rename fragment_uuid parameter to index_uuid (pylance 4.0.0 API change)
- Fix replace logic in create_scalar_index_internal to include removed_indices
- Handle 'not a directory' error in _load_existing_dataset
- Update test error message match for new RTREE index type
@XuQianJin-Stars XuQianJin-Stars requested a review from a team as a code owner April 11, 2026 08:03
@github-actions github-actions bot added the feat label Apr 11, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 11, 2026

Greptile Summary

This PR adapts the Lance integration to pylance 4.0.0 by renaming the fragment_uuidindex_uuid API parameter, populating removed_indices on replace to prevent a panic, handling the new "not a directory" error message in _load_existing_dataset, and updating the scalar-index test to include RTREE.

  • Version constraint mismatch: lance = [\"pylance>=0.39.0,<5.0.0\"] allows pylance 0.39.x, but the index_uuid keyword argument only exists in pylance 4.0.0+. Users with 0.39.x will hit TypeError at runtime; the lower bound should be >=4.0.0.

Confidence Score: 4/5

Safe to merge after correcting the version lower bound in pyproject.toml; all other findings are minor style/robustness concerns.

One P1 finding: the optional-dependency lower bound pylance>=0.39.0 is inconsistent with the index_uuid rename and will cause a runtime TypeError for any user on pylance 0.39.x. Remaining findings are P2 quality concerns that do not block correctness on pylance 4.0.0.

pyproject.toml — version lower bound needs to be raised to >=4.0.0.

Important Files Changed

Filename Overview
pyproject.toml Version lower bound set to pylance>=0.39.0 is incompatible with the index_uuid rename that only exists in pylance 4.0.0; should be >=4.0.0.
daft/io/lance/lance_scalar_index.py Renames fragment_uuidindex_uuid for pylance 4.0.0 API compatibility and adds removed_indices population logic; silent exception handling and stale dataset_version are minor concerns.
daft/io/lance/lance_data_sink.py Adds "not a directory" to the error-message allowlist in _load_existing_dataset to handle pylance 4.0.0's changed error format; straightforward and correct.
tests/io/lancedb/test_lancedb_scalar_index.py Updates error-message match to include RTREE (pylance 4.0.0); regex is now tightly coupled to a specific pylance error string and will break if pylance adds more index types.
uv.lock Lockfile updated to pin dev dependency to pylance==4.0.0 wheels; also updates lance-namespace-urllib3-client to 0.6.1 as part of the dependency chain refresh.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant create_scalar_index_internal
    participant FragmentIndexHandler
    participant lance_ds

    Caller->>create_scalar_index_internal: create_scalar_index(uri, column, replace=True)
    create_scalar_index_internal->>lance_ds: get_fragments()
    create_scalar_index_internal->>FragmentIndexHandler: __call__(fragment_ids)
    FragmentIndexHandler->>lance_ds: create_scalar_index(index_uuid=..., fragment_ids=...)
    note over FragmentIndexHandler,lance_ds: pylance 4.0.0: index_uuid (was fragment_uuid)
    create_scalar_index_internal->>lance_ds: merge_index_metadata(index_id)
    create_scalar_index_internal->>lance_ds: list_indices()
    note over create_scalar_index_internal: NEW: populate removed_indices for replace=True
    create_scalar_index_internal->>lance_ds: LanceDataset.commit(CreateIndex(new_indices, removed_indices))
Loading

Reviews (1): Last reviewed commit: "feat: support pylance 4.0.0" | Re-trigger Greptile

Comment thread pyproject.toml Outdated
huggingface = ["huggingface-hub<1.5.0", "datasets<4.6.0"]
iceberg = ["pyiceberg >= 0.7.0, <= 0.11.0, != 0.9.1, != 0.10.0"]
lance = ["pylance<0.40.0"]
lance = ["pylance>=0.39.0,<5.0.0"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Version lower-bound incompatible with renamed API

pylance>=0.39.0 allows versions that do not accept the index_uuid keyword argument introduced in pylance 4.0.0. The FragmentIndexHandler.__call__ now passes index_uuid=self.index_uuid to lance_ds.create_scalar_index(); on pylance 0.39.x (where the parameter was still called fragment_uuid) this raises TypeError: create_scalar_index() got an unexpected keyword argument 'index_uuid' at runtime. The lower bound should be raised to match the new API.

Suggested change
lance = ["pylance>=0.39.0,<5.0.0"]
lance = ["pylance>=4.0.0,<5.0.0"]

Comment thread daft/io/lance/lance_scalar_index.py Outdated
Comment on lines +229 to +231
except Exception:
# If we can't check existing indices, continue without removing
pass
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Silent failure re-triggers the panic being fixed

If list_indices() raises for any reason (network hiccup, API change, etc.), execution falls through with removed_indices = []. Committing a CreateIndex operation with an empty removed_indices while an index of the same name already exists was exactly the scenario that caused the pylance 4.0.0 panic, so swallowing the exception silently defeats the purpose of this block. Consider logging at warning level:

except Exception as e:
    logger.warning(
        "Could not fetch existing indices for removal; old index may not be cleaned up: %s", e
    )

Comment on lines 198 to 201
with pytest.raises(
NotImplementedError,
match=r'Only "BTREE", "BITMAP", "NGRAM", "ZONEMAP", "LABEL_LIST", or "INVERTED" or "BLOOMFILTER" are supported for scalar columns. Received INVALID',
match=r'Only "BTREE", "BITMAP", "NGRAM", "ZONEMAP", "LABEL_LIST", "INVERTED", "BLOOMFILTER" or "RTREE" are supported for scalar columns. Received INVALID',
):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Error-message match is pylance-version-specific

The regex now matches the exact error text emitted by pylance 4.0.0 (including "RTREE"). Whenever pylance adds or renames index types in a future release the match will fail. A narrower pattern that only captures the invariant part of the message is more robust:

Suggested change
with pytest.raises(
NotImplementedError,
match=r'Only "BTREE", "BITMAP", "NGRAM", "ZONEMAP", "LABEL_LIST", or "INVERTED" or "BLOOMFILTER" are supported for scalar columns. Received INVALID',
match=r'Only "BTREE", "BITMAP", "NGRAM", "ZONEMAP", "LABEL_LIST", "INVERTED", "BLOOMFILTER" or "RTREE" are supported for scalar columns. Received INVALID',
):
match=r'Received INVALID',

Comment on lines +219 to +228
removed_indices.append(
lance.Index(
uuid=idx["uuid"],
name=idx["name"],
fields=[lance_ds.schema.get_field_index(f) for f in idx["fields"]],
dataset_version=lance_ds.version,
fragment_ids=idx.get("fragment_ids", set()),
index_version=0,
)
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 dataset_version for removed indices uses current version instead of the index's creation version

lance_ds.version here is the version after merge_index_metadata was called, which is later than the version at which the existing (to-be-replaced) index was originally committed. Consider using the version from the index metadata if available:

dataset_version=idx.get("dataset_version", lance_ds.version),

- Bump pylance minimum version from 0.39.0 to 4.0.0 (index_uuid API requires 4.0+)
- Replace silent 'except Exception: pass' with warning logs
- Use index metadata's dataset_version instead of current version for removed_indices
- Relax error message match in test to avoid version-specific breakage
@madvart madvart requested a review from srilman April 14, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant