Conversation
Member
Author
|
Thanks for this @vineetbansal. I think this is a nice feature. The slight ambiguity is that this can only be used for jobs which report dictionaries/other objects, and you only want to access one attribute/item/field. In the future, it would be nice if this could be used as: @job
def make_str(s):
return s
@job
def capitalize(s):
return s.upper()
job1 = make_str("world")
job2 = capitalize(job1)
flow = Flow([job1, job2])I'm not sure how much trouble that will be to implement. I'm happy to merge this for now but I'm interested if this use case is also of interest for @Andrew-S-Rosen? |
Member
|
I agree that would be a nice addition as well. I have opened a new issue (am on mobile so can't edit it for additional clarity but can do so later). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resurrected from #823
Summary
This is essentially trying to resurrect the idea of
__getitem__access to aJobreturning the item from its.output, as in a previous PR, but limited in scope to avoid breaking changes.A use-case for this (included here as a test) is:
Context
While working with our package that uses
jobflow,prefect,parsland other workflow engines under the hood, we've recognized that flows are expressed succinctly as lookups on jobs:While the
@flowdecorator is a parallel effort that we hope can be merged in soon, this PR focuses only on thegetitempart.The existing PR on this from a couple of years ago was also trying to integrate attribute access to jobs and flows, which is a more overreaching change, and calls for more internal refactoring than what we feel might be needed. For example, one suggested change there:
would necessitate that all current and future attributes of
Jobs andOutputReferences be non-overlapping, which might be more trouble for development than is worth. A dict-lookup should not have the same disruptive effect from the point of view of the codebase, but will provide a useful shortcut for flow creation.Checklist
Before a pull request can be merged, the following items must be checked:
The easiest way to handle this is to run the following in the correct sequence on
your local machine. Start with running black on your new code. This will
automatically reformat your code to PEP8 conventions and removes most issues. Then run
pycodestyle, followed by flake8.
Run pydocstyle on your code.
type check your code.
Note that the CI system will run all the above checks. But it will be much more
efficient if you already fix most errors prior to submitting the PR. It is highly
recommended that you use the pre-commit hook provided in the repository. Simply
pip install pre-commitand thenpre-commit installand a check will be runprior to allowing commits.