Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specialize ExperimentAxisQuery.{obs,var} to pa.IntegerArray #243

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

ryan-williams
Copy link
Member

@ryan-williams ryan-williams commented Nov 1, 2024

Issue

  • SparseNDArray.read requires a SparseNDCoords arg. This can be a tuple of pa.IntegerArrays, but not a tuple of pa.Arrays.
  • We nevertheless pass pa.Arrays in two places:
    • ExperimentAxisQuery.X passes self._joinids.{obs,var}; both are pa.Arrays.
    • _fast_csr.read_csr passes {obs,var}_joinids; both are pa.Arrays.
  • In reality, the values are actually pa.IntegerArrays, so it's valid at runtime, but the types don't reflect that.
  • IntelliJ was warning me about it; not sure why mypy wasn't catching it.

Fix

Base automatically changed from rw/idx to main November 1, 2024 15:11
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@bkmartinjr bkmartinjr left a comment

Choose a reason for hiding this comment

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

LGTM

@ryan-williams ryan-williams merged commit 6243608 into main Nov 5, 2024
6 checks passed
@ryan-williams ryan-williams deleted the rw/i0 branch November 5, 2024 15:32
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.

3 participants