memgraph / mage

MAGE - Memgraph Advanced Graph Extensions :crystal_ball:
Apache License 2.0
246 stars 24 forks source link

[main < T load, parse] Implement xml.load and xml.parse #329

Closed mpintaric55334 closed 1 year ago

mpintaric55334 commented 1 year ago

Description

Please briefly explain the changes you made here.

Pull request type

Related issues

Delete if this PR doesn't resolve any issues. Link the issue if it does.

######################################

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

######################################

antepusic commented 1 year ago

@antoniofilipovic The tests use example XMLs from a third-party website. Thinking about reliability, I think it’s better to make our own example XMLs and host them on some Memgraph repo.

mpintaric55334 commented 1 year ago

@antepusic @antoniofilipovic shouldnt the map order always be same in tests?

antoniofilipovic commented 1 year ago

mple XMLs from a third-party website. Thinking about reliability, I think it’s better to make our own example XMLs and host them on some Memgraph repo.

@antepusic I wouldn't do that for now. If it starts failing, we can check then.

antoniofilipovic commented 1 year ago

About map order: The map contains elements from children, whose value is taken from the ElementTree parsed by .fromstring.

result[children_name] = [
    parse_element(child, simple) for child in children
]

After checking, I think their ordering is consistent so the tests are fine in that respect.

@antepusic so is PR ready to be merged or not?

antepusic commented 1 year ago

It’s ready then! 🚀