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

Added some comments and docstrings #242

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added some comments and docstrings #242

wants to merge 2 commits into from

Conversation

ahsimb
Copy link
Contributor

@ahsimb ahsimb commented Dec 20, 2024

No description provided.

@ahsimb ahsimb added the documentation User guides, tutorials, specifications label Dec 20, 2024
@ahsimb ahsimb self-assigned this Dec 20, 2024
@@ -13,6 +13,9 @@ class Dataset:

The TableLike refers to a database table containing the actual data that
can be used for instance in training or testing.

Q. A TableLike is basically a list of columns and a name. Why do we have
a separate list of columns here?
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK there was and is a separation of "ordinary" columns and "identifier columns" which may be handled specially in some cases? But the attribute columns could be superfluous in my understanding.

"""
This is a node of an ExecutionGraph.
Essentially, this is a node-level query handler factory. The query handler
itself is user-provided and so is this factory.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
itself is user-provided and so is this factory.
itself is user-provided and so are the factories implementing this interface.

Comment on lines +11 to +13
This is a type root for a class representing an input/output data for a customer
provided node-level query handler - SQLStageQueryHandler. The actual content of
the input/output is application specific.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This is a type root for a class representing an input/output data for a customer
provided node-level query handler - SQLStageQueryHandler. The actual content of
the input/output is application specific.
This is a type root for a class representing input/output data for a customer
provided node-level query handler extending class SQLStageQueryHandler. The actual content of
the input/output is application-specific.

@@ -98,6 +105,11 @@ def handle_result(
def _handle_finished_result(
self, result: Finish[SQLStageInputOutput]
) -> ResultHandlerReturnValue:
"""
We are done with the current node.
If there are successors we need to add the final output from this node to the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If there are successors we need to add the final output from this node to the
If there are successors, then we need to add the final output from this node to the

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation User guides, tutorials, specifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants