-
Notifications
You must be signed in to change notification settings - Fork 203
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
Updated glossary: clarified Future, AppFuture, and DataFuture; added … #3789
Conversation
docs/userguide/glossary.rst
Outdated
+===============+======================+========================+ | ||
| Represents | Function execution | File produced by an app| | ||
| Use Case | Track function call | Track data dependency | | ||
| Access Method | `.result()` | `.fetch()` | |
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.
can you give a link for .fetch()? did you generate this PR with an LLM?
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.
I have reviewed what you said and i wrote this manually.Thankyou for your feedback.Could you please review and approve the PR?
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.
Thanks @Ipshita29 for putting this together. We definitely need to explain App vs DataFutures better
I've listed a few ideas of other information to add based on where we explain DataFutures in the tutorial: https://parsl.readthedocs.io/en/stable/1-parsl-introduction.html#DataFutures
docs/userguide/glossary.rst
Outdated
Both types allow Parsl to manage and track asynchronous computations efficiently. | ||
|
||
Comparison: AppFuture vs. DataFuture | ||
------------------------------------ |
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.
Could you switch these underlines from "----" to "+++++" so that the documentation will make it at a lower level of header than "Future"
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.
I have updated the changes . Kindly review and Thankyou so much for your feedback.
A future is a placeholder for the result of a task that hasn't finished yet. Parsl provides two specialized types of futures: **AppFuture** and **DataFuture**. | ||
|
||
- **AppFuture** represents the result of an app (function execution) that runs in the background. You can retrieve the result using `.result()`. | ||
- **DataFuture** represents a **file-based dependency** produced by an app. You can retrieve it using `.fetch()`. |
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.
The fetch
here should also be replaced with result
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.
Maybe also mention that the DataFutures
are part of the outputs of an AppFuture, and that calling .result()
gives you a path of a file holding the data.
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.
Thankyou so much for your feedback I have made the changes further,Kindly review and merge my PR.
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.
fetch
in the above documentation is wrong. No such command exists in Parsl. Could you change it and also describe that DataFutures
are always created as part of an AppFuture
?
Comparison: AppFuture vs. DataFuture | ||
------------------------------------ | ||
|
||
The following table highlights the differences between AppFuture and DataFuture: |
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.
I'm not sure if the table adds much more over the bullet point list. Do you think the two provide different information?
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.
I have added a table because I thought it adds more readability and understanding. Let me know if you want me to do any changes.
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.
I'd suggest removing the table, as it duplicates information from the above paragraph and implies that the output of a Parsl function could be either App or Data. It's always App
closing as factually incorrect and possibly LLM generated. |
Description
This PR updates the glossary (
glossary.rst
) by clarifying the definitions of Future, AppFuture, and DataFuture.A comparison table has been added to improve clarity and provide a better understanding of their differences.
These changes aim to enhance the documentation for new users who may find these terms confusing.
Changed Behaviour
Fixes
Fixes # (issue number, if applicable)
Type of change