Skip to content

Conversation

evertlammerts
Copy link
Collaborator

Fixes #57

  • Created and curated new stubs
  • Added py.typed
  • Added mypy config and made it pass

Still todo:

  • Re-enable mypy tests. Through pytest and / or in CI?
  • Create a stub validity test?

Adding tests for the validity of stubs would be useful depending on what we can actually test:

  • We can test which symbols are exposed. If a function, class, constant etc is added or removed, then the output of a stub generator like pybind11-stubgen reflects that change. We can diff that output against our existing stubs and detect the change.
  • We can test changes in types that are automatically discovered by a stub generator, by keeping raw output from the generator in the repo and diffing that against stubgen runs during tests. This might produce false positives of course, when the stub generator changes its behavior.
  • Can we test whether changes to the manually curated stubs are valid? Mypy tests can catch mismatches if the changed symbol is used internally, but there's a lot it might miss. This is probably something a reviewer needs to be vigilant about.
  • Can we test whether changes to public symbols are reflected in the stubs if it's not a change the stub generator detects? Likely not.

I'm not completely sure yet how much effort the above deserves. I'll definitely add mypy checks and see where we go from there.

@evertlammerts
Copy link
Collaborator Author

@dsevilla can you double check that from_parquet and read_parquet are up to par now?

@dsevilla
Copy link
Contributor

dsevilla commented Oct 1, 2025

The signatures appear to be correct for both from_parquet and read_parquet. Two small issues, though:

  • The @pytyping.overload decorator only appears in the first function of the two overloads. I think it should be included in both.
  • The scripts have not been updated with this change of @pytyping.overload instead of just @overload. I suppose you used other tools, but it would be nice to check that later. Also, I believe the scripts that generate/change the wrapper functions generate correctly the overload for both functions.

Apart from that, correct wrt. these two functions.

@evertlammerts evertlammerts marked this pull request as ready for review October 1, 2025 17:53
@evertlammerts evertlammerts marked this pull request as draft October 1, 2025 21:38
@evertlammerts evertlammerts marked this pull request as ready for review October 2, 2025 10:25
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