Skip to content
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

Not able to reproduce lubridate values with vignette #189

Closed
jayqi opened this issue Feb 23, 2019 · 4 comments · Fixed by #181
Closed

Not able to reproduce lubridate values with vignette #189

jayqi opened this issue Feb 23, 2019 · 4 comments · Fixed by #181
Assignees

Comments

@jayqi
Copy link
Collaborator

jayqi commented Feb 23, 2019

We have the following copied example for lubridate in the vignette.

Function In-Subgraph Size Coverage Ratio Total Lines
divide_period_by_period 39 1 2
days 22 1 1
check_duration 15 0 1
as.POSIXt 13 0 1
eweeks 13 0 2
check_interval 12 0 11
date<- 12 NA NA
add_months 10 1 4
ceil_multi_unit 10 1 1
am 6 1 1

I am not able to reproduce this result.


Installing the current CRAN version (0.3.2), and using the version of lubridate specified in the vignette, I get the following for the top 10 nodes by inSubgraphSize:

            node inSubgraphSize coverageRatio totalLines
 1:        month             82             1          1
 2:           tz             80             1          1
 3: reclass_date             69             1          1
 4:         date             68             1          1
 5:      is.Date             61             1          1
 6:    is.POSIXt             58             1          1
 7:         wday             57             1          1
 8:  .deprecated             56             0         10
 9:   is.POSIXct             56             1          1
10:      as_date             53             1          1

If I look at the same 10 nodes as the vignette, I get

                       node inSubgraphSize coverageRatio totalLines
 1: divide_period_by_period              1             1          2
 2:                    days             10             1          1
 3:          check_duration              1             0          3
 4:               as.POSIXt              1             0          1
 5:                  eweeks              1             0          2
 6:          check_interval              1             0         13
 7:                  date<-              1            NA         NA
 8:              add_months              3             1          4
 9:         ceil_multi_unit              5             1          1
10:                      am              2             1          1

I suspect what happened was two things:

  1. Because we added support for non-exported functions (per FunctionReporter handling of non-exported internal functions #122, in PR FunctionReporter to handle Non-exported Functions #125), there are now a lot more connections in the graph, so the top ten nodes by inSubgraphSize are much bigger.
  2. The same ten nodes have way lower inSubgraphSize because of bug Network Statistics are off #165 that was fixed in PR Bugfix Nodes Table Merging #166. In fact, divide_period_by_period is even mentioned in that issue.

I think we should just replace this table in the vignette with the new correct top 10 nodes. I will do it as part of PR #181.

@jayqi
Copy link
Collaborator Author

jayqi commented Feb 23, 2019

Additionally, because of the new non-exported functions from #125, the Jaccard Similarity example looking at groups with identical out-neighborhoods returns 20 groups instead of seven. It looks clunkier, but I'm just going to paste it in for now in #181 in order to have a more faithful reproduction of what the vignette is doing.

@jayqi jayqi self-assigned this Feb 23, 2019
@bburns632
Copy link
Collaborator

bburns632 commented Feb 23, 2019 via email

@jayqi
Copy link
Collaborator Author

jayqi commented Feb 23, 2019

I'm cool with keeping the results static. I was just saying we should update the static results with the new values that you'd get with our bug-fixed-new-functionality version of pkgnet. At a minimum, the old static results in the vignette are just plain wrong because of bug #165.

@bburns632
Copy link
Collaborator

bburns632 commented Feb 23, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants