Nick/shape support #348
Merged
Nick/shape support #348
Conversation
This file contains 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
* Coerce a broader class of things into a shape tuple * Use math prob on int tuple instead of np.prod to avoid overflow when possible
* pyttb_utils is more internal so can probably be more specific
|
@ntjohnson1 It looks like the regression tests passed when this PR was waiting to be merged, but then they failed due to mypy errors when the GitHub actions were run after the PR merge. I confused as to why that would happen. Perhaps I missed a step during the review and merge of the PR. I now have broken the dev pipeline pyttb in two different ways today. Ugh! |
|
Between when I posted my PR and you merged it a Numpy release came out which subtly improved their typing support. I fixed it in the other PR so we should be set for the moment. |
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.
This should make our shape arguments more flexible.
This provides machinery for simplifying when we want a vector (or scalar) of items. I didn't distinguish between an index array or a data array so technically this makes our typing slightly less specific. If that causes immediate concern can specify. Otherwise I think this is better since we were over constrained below (List[int] when really an iterable or numpy array of integers would be fine).
Mostly just a matter of defining the type, a parsing utility and pushing things through to make mpypy happy
Resolves #305 along the way.
I also realized we didn't set the flag to assert when vals or subs check failed. I turned the flag on but that required fixing some of the tutorial. Since this PR is all about making our interface nicer I think a follow up might be to make vals just be an arbitray OneDArray, I don't think we really need it to be a column array (or if we do we could probably adjust it internally).
📚 Documentation preview 📚: https://pyttb--348.org.readthedocs.build/en/348/