Fixes several bugs in reader_dispatch #108
Merged
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
ilumsden
added
area-thicket
…unctions correctly
ilumsden
force-pushed
the
improve_reader_dispatch
branch
from
b0908e9 to
9c10e19
Compare
michaelmckinsey1
approved these changes
Nov 2, 2023
|
In regards to type checking, I found this reference that enforces the new solution implemented here. Most importantly, |
slabasan
approved these changes
Nov 8, 2023
Yejashi
pushed a commit
to TauferLab/thicket
that referenced
this pull request
Mar 6, 2024
…unctions correctly (LLNL#108)
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 PR fixes two bugs in
Thicket.reader_dispatch.First, it fixes type checking in the function. Currently,
reader_dispatchusestype(var) = expected_type. This syntax may work in some cases. However, there is no guarantee it will work for all types, and the Python Software Foundation has reserved the right to completely invalidate this syntax in the future. This PR fixes this type checking by switching it to the more correctisinstance(var, expected_type)syntax.Additionally, this PR fixes a bug in
Thicket.reader_dispatchthat would cause extra positional and keyword arguments to not be forwarded to the corresponding Hatchet function if the user passed in a list or tuple of filenames. This is particularly problematic forThicket.from_caliperbecause it means thequeryargument is not forwarded to Hatchet, causingThicket.from_caliperto fail in all cases.