Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[master < xml_module] Write docs for xml module #1026

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mpintaric55334
Copy link
Contributor

Description

Please briefly explain the changes you made here.

I wrote a dosc page for xml_module QM in MAGE.

Pull request type

Please delete options that are not relevant and check the ones that are.

  • New documentation page

Related PRs and issues

PR this doc page is related to:

memgraph/mage#329

Closes (link to issue):

Checklist:

  • I checked all content with Grammarly
  • I performed a self-review of my code
  • I made corresponding changes to the rest of the documentation
  • The build passes locally
  • My changes generate no new warnings or errors

@mpintaric55334 mpintaric55334 added the status: ready PR is ready for review label Sep 8, 2023
@mpintaric55334 mpintaric55334 self-assigned this Sep 8, 2023
#### Input:

- `xml_input: string` ➡ input XML string.
- `simple: bool (default = false)` ➡ bool used for specifying whether simple mode should be used.
Copy link
Collaborator

@vpavicic vpavicic Sep 11, 2023

Choose a reason for hiding this comment

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

link to the explanation of the simple mode

Copy link
Collaborator

Choose a reason for hiding this comment

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

that heading is called simple configuration, should this explanation also use the word configuration so i can connect these two


#### Simple configuration explanation

For details about simple configuration, go to [**Simple configuration explanation in parse function**](#simple-configuration-explanation).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I would add this explanation to the introduction. SOmething like -you can choose to execute the procedure in a simple and then explain what it means... so when i am already reading about procedures i know what it is + no header necessary, no repetition in each procedure


#### XPath

This procedure supports the usage of XPath expressions. Since the module is implemented in Python, XPath expressions should follow, and are limited to the XPath syntax explained here [**XML python docs**](https://docs.python.org/3/library/xml.etree.elementtree.html#xpath-support). XPath implemented this way cannot use absolute paths, so one of these 3 prefixes must be used to avoid errors: `. .. *`. The current node is the root node.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why isn't this in the intro part of the procedure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because XPath is not the main purpose of the function, its more a side addition for users who want to use it

Copy link
Collaborator

@vpavicic vpavicic left a comment

Choose a reason for hiding this comment

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

see comments

@vpavicic vpavicic added status: change PR reviewed - needs changes and removed status: ready PR is ready for review labels Sep 11, 2023
@mpintaric55334 mpintaric55334 added status: ready PR is ready for review and removed status: change PR reviewed - needs changes labels Sep 11, 2023
@vpavicic vpavicic added status: ship it PR approved added-to-new-docs and removed status: ready PR is ready for review subgraph labels Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants