-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
fix: Normalising Children and New Nodes #111
Changes from 4 commits
36fea9a
3eea818
79b7e32
c03f3ca
d0beb2c
521ad50
a8204a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -18,12 +18,16 @@ def create_node(label, entry, main_language_code): | |||||||||||||||||
""" | ||||||||||||||||||
Helper function used for creating a node with given id and label | ||||||||||||||||||
""" | ||||||||||||||||||
#Normalising new Node ID | ||||||||||||||||||
|
||||||||||||||||||
normalised_entry = normalizing(entry, main_language_code).split(':', 1)[1] | ||||||||||||||||||
|
||||||||||||||||||
query = [f"""CREATE (n:{label})\n"""] | ||||||||||||||||||
params = {"id": entry} | ||||||||||||||||||
params = {"id": normalised_entry} | ||||||||||||||||||
|
||||||||||||||||||
# Build all basic keys of a node | ||||||||||||||||||
if (label == "ENTRY"): | ||||||||||||||||||
canonical_tag = entry.split(":", 1)[1] | ||||||||||||||||||
canonical_tag = normalised_entry.split(":", 1)[1] | ||||||||||||||||||
query.append(f""" SET n.main_language = $main_language_code """) # Required for only an entry | ||||||||||||||||||
params["main_language_code"] = main_language_code | ||||||||||||||||||
else: | ||||||||||||||||||
|
@@ -173,13 +177,27 @@ def update_nodes(label, entry, new_node_keys): | |||||||||||||||||
continue | ||||||||||||||||||
query.append(f"""\nREMOVE n.{key}\n""") | ||||||||||||||||||
|
||||||||||||||||||
#Normalising Node to be Updated | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
normalised_new_node_keys = {} | ||||||||||||||||||
for keys in new_node_keys.keys(): | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should name it |
||||||||||||||||||
if keys.startswith("tags") and not keys.endswith("str"): | ||||||||||||||||||
if "ids" not in keys: | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add the "_" to make the condition even more specific and explicit
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense |
||||||||||||||||||
keys_language_code = keys.split('_', 1)[1] | ||||||||||||||||||
normalised_value = [] | ||||||||||||||||||
for values in new_node_keys[keys]: | ||||||||||||||||||
normalised_value.append(normalizing(values, keys_language_code)) | ||||||||||||||||||
normalised_new_node_keys[keys] = normalised_value | ||||||||||||||||||
aadarsh-ram marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
normalised_new_node_keys["tags_ids_"+keys_language_code] = normalised_value | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add a else to make explicit it's intended
Suggested change
|
||||||||||||||||||
else: | ||||||||||||||||||
normalised_new_node_keys[keys] = new_node_keys[keys] | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
# Update keys | ||||||||||||||||||
for key in new_node_keys.keys(): | ||||||||||||||||||
for key in normalised_new_node_keys.keys(): | ||||||||||||||||||
query.append(f"""\nSET n.{key} = ${key}\n""") | ||||||||||||||||||
|
||||||||||||||||||
query.append(f"""RETURN n""") | ||||||||||||||||||
|
||||||||||||||||||
params = dict(new_node_keys, id=entry) | ||||||||||||||||||
params = dict(normalised_new_node_keys, id=entry) | ||||||||||||||||||
result = get_current_transaction().run(" ".join(query), params) | ||||||||||||||||||
return result | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -206,7 +224,13 @@ def update_node_children(entry, new_children_ids): | |||||||||||||||||
existing_ids = [record['child.id'] for record in get_current_transaction().run(query, ids=list(added_children))] | ||||||||||||||||||
to_create = added_children - set(existing_ids) | ||||||||||||||||||
|
||||||||||||||||||
for child in to_create: | ||||||||||||||||||
#Normalising new children node ID | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pep8 ask for space after the
Suggested change
|
||||||||||||||||||
normalised_added_children = set() | ||||||||||||||||||
for child_node in to_create: | ||||||||||||||||||
child_language_code, new_child_name = child_node.split(":",1) | ||||||||||||||||||
normalised_added_children.add(normalizing(new_child_name,child_language_code)) | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have to put back language code !
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but see below because I propose you to remove this part. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As create_node is used below and it already normalize id I don't think we need this (and this avoid duplication of code). Instead I propose to build normalised_added_children while creating nodes (see below) |
||||||||||||||||||
|
||||||||||||||||||
for child in normalised_added_children: | ||||||||||||||||||
main_language_code = child.split(":", 1)[0] | ||||||||||||||||||
create_node("ENTRY", child, main_language_code) | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I propose to instead do
Suggested change
By the way we can use a shorter name like created_ids instead of normalised_added_children |
||||||||||||||||||
|
||||||||||||||||||
|
@@ -215,7 +239,7 @@ def update_node_children(entry, new_children_ids): | |||||||||||||||||
|
||||||||||||||||||
# Stores result of last query executed | ||||||||||||||||||
result = [] | ||||||||||||||||||
for child in added_children: | ||||||||||||||||||
for child in normalised_added_children: | ||||||||||||||||||
# Create new relationships if it doesn't exist | ||||||||||||||||||
query = f""" | ||||||||||||||||||
MATCH (parent:ENTRY), (new_child:ENTRY) WHERE parent.id = $id AND new_child.id = $child | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not make the split here, should you ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... yup.