Skip to content
This repository has been archived by the owner on Nov 11, 2018. It is now read-only.

nest.rollup should create {key, value} for leaf entries. #3

Closed
mbostock opened this issue Apr 21, 2016 · 9 comments
Closed

nest.rollup should create {key, value} for leaf entries. #3

mbostock opened this issue Apr 21, 2016 · 9 comments
Assignees

Comments

@mbostock
Copy link
Member

This makes it easier to disambiguate leaf nodes from parent nodes.

@mbostock mbostock self-assigned this Apr 21, 2016
@mbostock mbostock changed the title nest.rollup + nest.entries should create {key, value} for leaves. nest.rollup should create {key, value} for leaf entries. May 14, 2016
@mbostock
Copy link
Member Author

Perhaps nest.entries should create {key, children} by default, too. Then it would be default-compatible with d3.hierarchy.

@syntagmatic
Copy link

I think it's best to keep the behavior of d3.nest as it is-- at least key/values for the internal node keys. It's a simple, successful function with many examples and tutorials out there.

The referenced issue could resolved with a new convenience function in d3-hierarchy that has an API similar d3.nest but returns a hierarchy object. The new function would be a sibling to stratify.

@mbostock
Copy link
Member Author

mbostock commented May 15, 2016

I don’t want to introduce a new operator; that’s a pretty big addition to the API when there’s already a reasonable way to turn the result of nest.entries into a d3.hierarchy. We’re just talking about turning this:

var root = d3.hierarchy({values: nest.entries(data)}, function(d) { return d.values; })
    .sum(function(d) { return d.values; });

Into this:

var root = d3.hierarchy({children: nest.entries(data)})
    .sum(function(d) { return d.value; });

Or, if we just change the behavior of nest.rollup and don’t adopt entry.children:

var root = d3.hierarchy({values: nest.entries(data)}, function(d) { return d.values; })
    .sum(function(d) { return d.value; });

Name changes are the most trivial of incompatibilities. We shouldn’t needlessly rename things, but if changing entry.values to entry.children makes things easier, it might be worth it. Certainly there are a lot of other API changes in D3 4.0 that I believe are worth it; we shouldn’t needlessly bind ourselves to past API decisions if we’ve learned something since.

It is interesting to consider whether d3.nest should return a d3.hierarchy instance directly. That would suggest moving d3.nest to the d3-hierarchy module, or perhaps better, moving d3.hierarchy and d3.stratify into d3-collection (since these are just operations on data and not hierarchical layouts). That would offer some nice improvements, like addressing #4 because d3.hierarchy provides visit methods, but you also lose something because nests are specialized hierarchies where each child is uniquely identified by a string key. It’s sometimes convenient to represent them as nested maps.

So, I feel like it’s worth considering renaming entry.values to entry.children (and having nest.rollup replace entry.children with entry.value), but I’m not currently in favor of more substantial changes.

mbostock added a commit that referenced this issue May 15, 2016
Also changes nest.rollup to return entry.value.

Fixes #3.
@mbostock
Copy link
Member Author

I’ve implemented both approaches in #6 and #7. I guess I’m leaning towards #6 to be conservative now.

@mbostock
Copy link
Member Author

Merged #6.

@PatriciaW43
Copy link

I'm having problems with d3.nest. Sometimes rollup returns key/value and sometimes key/values. It appears that the first time it is invoked it results in key/values then key/value. This means that the program works correctly the first time but then it returns key/value and fails.

@mbostock
Copy link
Member Author

mbostock commented Nov 4, 2017

@PatriciaW43 nest.rollup always produces leaf entries with entry.value rather than entry.values. Please see my guide on reporting an issue.

@PatriciaW43
Copy link

I wasn't sure how to report an issue so I created a Gist at gist:fe81b673a90a3cd5f32b7aceca9559c0

@bplmp
Copy link

bplmp commented May 16, 2018

For me nest.rollup is also producing entry.values on leaf entries.

See this example, which is linked from d3-collection's readme: http://bl.ocks.org/phoebebright/raw/3176159/

var nested_data = d3.nest()
.key(function(d) { return d.status; })
.key(function(d) { return d.priority; })
.rollup(function(leaves) { return leaves.length; })
.entries(csv_data);

Returns:

[
   {
      "key": "Complete",
      "values": [
         {
            "key": "MUST",
            "values": 5
         },
         {
            "key": "SHOULD",
            "values": 5
         },
         {
            "key": "COULD",
            "values": 1
         }
      ]
   },
   {
      "key": "In Progress",
      "values": [
         {
            "key": "MUST",
            "values": 6
         },
         {
            "key": "SHOULD",
            "values": 1
         },
         {
            "key": "WISH",
            "values": 1
         }
      ]
   },
   {
      "key": "Not Started",
      "values": [
         {
            "key": "MUST",
            "values": 9
         },
         {
            "key": "SHOULD",
            "values": 3
         },
         {
            "key": "COULD",
            "values": 3
         },
         {
            "key": "WISH",
            "values": 1
         }
      ]
   }
]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants