-
Notifications
You must be signed in to change notification settings - Fork 31
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
subset
and merge
for VarInfo
(clean version)
#544
Merged
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
028a81a
added `subset` which can extract a subset of the varinfo
torfjelde caa6e25
added testing of `subset` for `VarInfo`
torfjelde cac7fa8
formatting
torfjelde 5e41c4f
added implementation of `merge` for `VarInfo` and tests for it
torfjelde d5a2631
more tests
torfjelde 0ade696
formatting
torfjelde db21844
improved merge_metadata for NamedTuple inputs
torfjelde 1dbca4c
added proper handling of the `vals` in `subset`
torfjelde b67288f
added docs for `subset` and `merge`
torfjelde e43029e
added `subset` and `merge` to documentation
torfjelde cd4033d
formatting
torfjelde 8f47dfe
made merge and subset part of the AbstractVarInfo interface
torfjelde aba9008
added implementations `subset` and `merge` for `SimpleVarInfo`
torfjelde 3b621ae
follow standard merge semantics where the right one takes precedence
torfjelde 2c2c90b
added proper testing of merge and subset for SimpleVarInfo too
torfjelde 5c1ece3
forgotten inclusion in previous commit
torfjelde cfff96c
Update src/simple_varinfo.jl
torfjelde ed5d948
remove two-argument impl of merge
torfjelde 00c36cf
formatting
torfjelde cf02816
forgot to add more formatting
torfjelde d02cb61
Merge branch 'master' into torfjelde/subset-and-merge
torfjelde 7f01ada
removed 2-arg version of merge for abstract varinfo in favour of 3-ar…
torfjelde 14105e0
allow inclusion of threadsafe varinfo in setup_varinfos
torfjelde c164d32
more tests for thread safe varinfo
torfjelde 743162a
bugfixes for link and invlink methods when using thread safe varinfo
torfjelde dc9ad94
attempt at fixing docs
torfjelde 2f320e6
fixed missing test coverage
torfjelde d3a9b56
formatting
torfjelde File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ export AbstractVarInfo, | |
SimpleVarInfo, | ||
push!!, | ||
empty!!, | ||
subset, | ||
getlogp, | ||
setlogp!!, | ||
acclogp!!, | ||
|
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we would not already have so many
getindex
methods, I would have thought thatgetindex
would be a natural name for this function. But maybe it's still an option?Then we could have
getindex(::AbstractVarInfo, ::AbstractVector{<:VarName}) -> AbstractVarInfo
andgetindex(::T, ::VarName) -> typeof_varname_variate
, similar to[1,2,3][[1,3]] = [1, 3]
and[1,2,3][2] = 2
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd really like this yes, but I also really don't want to touch
getindex
in this codebase 😅Happy to make this a long-term goal or something though!