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

Add support for multi-labeled nodes #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jeffreylovitz
Copy link
Contributor

This PR allows nodes to be created and parsed with an array of label string as well as with a label string (for backwards compatibility).

This should not break existing functionality, but adds the ability to have multiple labels on a given node.

@jeffreylovitz jeffreylovitz added the enhancement New feature or request label Oct 21, 2021
@jeffreylovitz jeffreylovitz requested a review from leibale October 21, 2021 18:06
@jeffreylovitz jeffreylovitz self-assigned this Oct 21, 2021
let properties = await this.parseEntityProperties(cell[2]);
let node = new Node(label, properties);
let node = new Node(labels, properties);
Copy link

Choose a reason for hiding this comment

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

'let' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

if (cell[1].length == 1) {
labels = await this.parseNodeLabel(cell[1][0]);
} else {
labels = await Promise.all(cell[1].map(x => this.parseNodeLabel(x)));
Copy link

Choose a reason for hiding this comment

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

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
Missing semicolon.

let node_id = cell[0];
var labels = undefined;
if (cell[1].length == 1) {
labels = await this.parseNodeLabel(cell[1][0]);
Copy link

Choose a reason for hiding this comment

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

Missing semicolon.

// [[name, value, value type] X N]

let node_id = cell[0];
var labels = undefined;
Copy link

Choose a reason for hiding this comment

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

It's not necessary to initialize 'labels' to 'undefined'.

// [label string offset (integer) X N],
// [[name, value, value type] X N]

let node_id = cell[0];
Copy link

Choose a reason for hiding this comment

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

'let' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

* @param {object[]} cell raw node representation.
* @returns {Promise<Node>} Node object.
*/
async parseNode(cell) {
Copy link

Choose a reason for hiding this comment

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

Class properties must be methods. Expected '(' but instead saw 'parseNode'.
Duplicate class method 'async'.

// will try to get the right label for at most 10 times
var tries = 0;
while (label == undefined && tries < 10) {
label = await this._graph.fetchAndGetLabel(cell[1][0]);
label = await this._graph.fetchAndGetLabel(label_idx);
Copy link

Choose a reason for hiding this comment

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

Missing semicolon.

let node_id = cell[0];
let label = this._graph.getLabel(cell[1][0]);
async parseNodeLabel(label_idx) {
let label = this._graph.getLabel(label_idx);
Copy link

Choose a reason for hiding this comment

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

'let' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).


let node_id = cell[0];
let label = this._graph.getLabel(cell[1][0]);
async parseNodeLabel(label_idx) {
Copy link

Choose a reason for hiding this comment

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

Class properties must be methods. Expected '(' but instead saw 'parseNodeLabel'.
Duplicate class method 'async'.

@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #82 (6bab604) into master (312f4ea) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #82   +/-   ##
=======================================
  Coverage   90.74%   90.74%           
=======================================
  Files           6        6           
  Lines         108      108           
  Branches        6        6           
=======================================
  Hits           98       98           
  Misses         10       10           
Impacted Files Coverage Δ
src/node.js 77.77% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 312f4ea...6bab604. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant