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.

Sign up for GitHub

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

Jump to bottom

Major Refactor to GraphReporters #181

Merged
merged 16 commits into from
Mar 12, 2019
Merged

Major Refactor to GraphReporters #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c16cc7b
AbstractGraph and DirectedGraph classes
jayqi Feb 22, 2019
2605327
Refactored graph reporters to use DirectedGraph and cleaned up interf…
jayqi Feb 22, 2019
8f2509c
Reorganized unit tests
jayqi Feb 22, 2019
a1cd6c3
Updated vigentte to work with calculate_default_measures. Also fixes …
jayqi Feb 23, 2019
eb76fe6
Roxygen documentation for DirectedGraph
jayqi Feb 23, 2019
556243a
Changed default and available measures to active instead of methods
jayqi Feb 23, 2019
a975944
Fixed annoyingly restrictive roxygen issue
jayqi Feb 23, 2019
da290cc
Updated NEWS.md, log message polish
jayqi Feb 23, 2019
ced3fd9
Fixed minor issues with vignette from R CMD check
jayqi Feb 23, 2019
e233ee5
Standardize on 'dependency' and 'reverse dependency'
jayqi Feb 28, 2019
a40c49d
Hide active bindings for R6 class stuff in FunctionReporter
jayqi Mar 2, 2019
d690d7a
DT importFrom
jayqi Mar 2, 2019
eb56b1c
Polish
jayqi Mar 2, 2019
a86e25c
Renamed recursiveDeps/recursiveRevDeps to numRecursiveDeps/numRecursi…
jayqi Mar 2, 2019
09b7cfb
Added ignore entries for vignette stuff
jayqi Mar 2, 2019
3e8ac75
numRecursiveDeps, ... in NEWS.md
jayqi Mar 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .Rbuildignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
# produced vignettes
vignettes/*\.html
vignettes/*\.pdf
^Meta$
^doc$

# Temporary files created by R markdown
.*\.utf8\.md
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ pkgnet.Rcheck/*

# Compiled vignette
inst/doc/*
Meta/*
doc/*

# aspell stuff
**/*.bak
15 changes: 8 additions & 7 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,21 @@ importFrom(futile.logger,flog.threshold)
importFrom(futile.logger,flog.warn)
importFrom(futile.logger,logger.options)
importFrom(igraph,V)
importFrom(igraph,centralization.betweenness)
importFrom(igraph,centralization.closeness)
importFrom(igraph,centralization.degree)
importFrom(igraph,authority_score)
importFrom(igraph,betweenness)
importFrom(igraph,centr_betw_tmax)
importFrom(igraph,centr_clo_tmax)
importFrom(igraph,centr_degree_tmax)
importFrom(igraph,centralize)
importFrom(igraph,closeness)
importFrom(igraph,degree)
importFrom(igraph,graph.edgelist)
importFrom(igraph,graph_from_edgelist)
importFrom(igraph,hub_score)
importFrom(igraph,layout_as_tree)
importFrom(igraph,layout_in_circle)
importFrom(igraph,make_empty_graph)
importFrom(igraph,neighborhood.size)
importFrom(igraph,page_rank)
importFrom(igraph,vcount)
importFrom(igraph,vertex)
importFrom(igraph,vertex.attributes)
importFrom(knitr,opts_chunk)
importFrom(magrittr,"%>%")
importFrom(methods,formalArgs)
Expand Down
23 changes: 17 additions & 6 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
# pkgnet 0.3.2.9999 (current dev)

## NEW FEATURES
None

* Objects of new `DirectedGraph` class now slot into the `pkg_graph` field of network reporters. These objects encapsulate the graph modeling of networks and have a more expressive set of methods for analysis. Check out the full documentation with `?DirectedGraph`. ([#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))
* Use `pkg_graph$node_measures` and `pkg_graph$graph_measures` to respectively calculate node-level and graph-level measures.
* Use `pkg_graph$default_node_measures` and `pkg_graph$default_graph_measures` to see the measures calculated by default.
* Use `pkg_graph$available_node_measures` and `pkg_graph$available_graph_measures` to see the all supported measures.
* The igraph object is now instead available at `pkg_graph$igraph`.

## CHANGES
None

* Standardizing on the language "dependency" and "reverse dependency" to describe the directed graph relationships in the package. This completes the change introduced in v0.3.0 where edge direction convention was set to point in the direction of dependency. So this means that "depend on" follows the edge arrow direction, and "reverse depends on" is reverse edge arrow direction. ([#191](https://github.com/UptakeOpenSource/pkgnet/issues/106), [#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))
jayqi marked this conversation as resolved.
Show resolved Hide resolved
* `outSubgraphSize` and `inSubgraphSize` have been replaced with `numRecursiveDeps` and `numRecursiveRevDeps`, which are the former minus one (by not counting the node itself). ([#191](https://github.com/UptakeOpenSource/pkgnet/issues/106), [#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))
* Per the new `DirectedGraph` feature, reporters' `pkg_graph` field now contain an object of new `DirectedGraph` class. Previously it held an igraph object. This igraph object is now instead available at `pkg_graph$igraph`. See NEW FEATURES section for other details about the new `pkg_graph` object. ([#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))
* Default measures now exist for each reporter. These can be calculated with the
new method `calculate_default_measures` on reporters. ([#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))
* The report from `CreatePackageReport` will now only show default measures.
* Reporters now only allow packages to be set once. To report on a new package, please instantiate a new instance of the reporter of interest. ([#106](https://github.com/UptakeOpenSource/pkgnet/issues/106), [#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))
* The report from `CreatePackageReport` now prints the version of pkgnet used at the bottom. ([#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))

## BUG FIXES
None
* Static outputs shown in the vignette that were outdated have been updated. ([#189](https://github.com/UptakeOpenSource/pkgnet/issues/189), [#181](https://github.com/UptakeOpenSource/pkgnet/pull/181))

# pkgnet 0.3.2

Expand All @@ -19,7 +32,7 @@ None
* Revised unit test setup and teardown files to enable devtools::test() to work as well as CRAN server testing ([#167](https://github.com/UptakeOpenSource/pkgnet/pull/167))

## BUG FIXES
* Corrected node statisitcs table merging error ([#165](https://github.com/UptakeOpenSource/pkgnet/issues/165), [#166](https://github.com/UptakeOpenSource/pkgnet/pull/166))
* Corrected node statistics table merging error ([#165](https://github.com/UptakeOpenSource/pkgnet/issues/165), [#166](https://github.com/UptakeOpenSource/pkgnet/pull/166))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you make this a separate PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? I don't want to screw with git to undo committing this. I also disagree that this kind of thing needs to be a separate PR. There is no discussion to be had, and setting the bar that high for fixing this kind of small typo makes it less likely that they get fixed. This file has no code and fixing this has no risk of breaking anything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"screw with git to undo committing this" --> that is not what I'm recommending. If you made another PR it would get merged immediately (since it's such a small and non-controversial change) and when you rebased this PR to master the diff would disappear with no conflict.

It's fine for now because it seems this PR is close to being merged anyway, but in general I disagree with "setting the bar that high for fixing this kind of small typo makes it less likely that they get fixed". I dislike the pattern of long-lived large PRs picking up other small, unrelated changes because it means that anyone else doing development on the project will be waiting on those changes unnecessarily.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that's fine. I have had some bad experiences in the past where the rebase still gives me conflicts, but maybe that was just bad luck and I'm overcompensating.

* Added a NAMESPACE entry for knitr to suppress warning on CRAN server checks ([#168](https://github.com/UptakeOpenSource/pkgnet/pull/168))

# pkgnet 0.3.1
Expand Down Expand Up @@ -53,5 +66,3 @@ None

## BUG FIXES
* Rendering of the table in Function Network tab. ([#136](https://github.com/UptakeOpenSource/pkgnet/issues/136), [#138](https://github.com/UptakeOpenSource/pkgnet/pull/138))

<!--- Start of NEWS.md --->
Loading