Add size_by argument to model_tree()
#720
Open
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
- Allows users to control the node sizing by a particular column present in `.log_df`
- Important to note that `ofv` cant be used without first calling `add_summary()`, which is consistent with the previous behavior of `color_by` and `include_info`
- This argument does support logical columns, however I noticed that the normalized sizing can differ here depending when the first `TRUE` value originates. If it's the first value, all the nodes appear smaller. Anywhere else and the "base size" is larger. This observation is likely true for numeric columns too, though is less apparent when the values are all closer to each other.
- `collapsibleTree::collapsibleTreeNetwork` is only intended to support numeric columns for sizing, so I opted to map these values to: TRUE=5, FALSE=1, NA=3 for the time being. We may want to revisit NA values for both numeric and logical columns
- Added examples to docs
- While the implementation and tests are done, I think we still want to revist the NA handling, which also impacts the "start" node size.
- While updating the vignette, I noticed that the size_by and color_by arguments occasionally didnt line up for numeric columns. Numeric columns need to be sorted first so the right colors are assigned. This was previously "tested" using the character 'run' column, though was more observable when testing against objective function values (which dont necessarily appear in ascending/descending order)
- uses leafCount, which makes the nodes containing more child nodes, larger
- Improve formatting of code in some cases
- They were always NA previously
- Adds additional tests when required or specified columns are missing - doc adjustments
- If color_by or size_by is specified, ensure these columns are also part of the tooltip. This avoids the need for specifying new columns (e.g., 'Out of Date') in both the tooltip and color_by
- doc and text adjustments - Revert change from previous commit (comment it out) until a decision has been made.
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.
Core new feature:
size_byAllows users to control the node sizing by a particular column present in
.log_dfImportant to note that
ofvcant be used without first callingadd_summary(), which is consistent with the previous behavior ofcolor_byandinclude_info.This argument does support logical columns, however I noticed that the normalized sizing can differ here depending when the first
TRUEvalue originates. If it's the first value, all the nodes appear smaller. Anywhere else and the "base size" is larger. This observation is likely true for numeric columns too, though is less apparent when the values are all closer to each other.collapsibleTree::collapsibleTreeNetworkis only intended to support numeric columns for sizing, so I opted to map these values to:TRUE=5,FALSE=1,NA=3for the time being. We may want to revisit NA values for both numeric and logical columnsOther changes
font_sizeargument. While updating the vignette I foresaw a future request to adjust this, and it was easy enough to pass tocollapsibleTree::collapsibleTreeNetwork(with one other change)