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

Docs: Further clarification on how serialization impacts where imports need to be #2918

Closed
Andrew-S-Rosen opened this issue Oct 16, 2023 · 2 comments · Fixed by #2919
Closed

Comments

@Andrew-S-Rosen
Copy link
Contributor

Andrew-S-Rosen commented Oct 16, 2023

Jotting this down from Slack so I/we don't forget:

Me:

The current docs as-is are kind of confusing with regards to the need to put import statements in PythonApp definitions. there's some pretty strong-worded statements about always needing to do so, but that's not the case (e.g. if you're making a package that will be installable). i'll give this some thought too. I think Logan Ward and I discussed this at one point

@WardLT:

Yea, we really do need to clarify how serialization works to explain that

@benclifford
Copy link
Collaborator

I'm mostly interested in cases where having too many imports of this form is actively harmful

@Andrew-S-Rosen
Copy link
Contributor Author

Andrew-S-Rosen commented Oct 16, 2023

@benclifford: I can provide some background. When I was first exploring which workflow engine to consider for my Python package, I was looking for a package where I didn't really have to change how I wrote my code --- just decorate and move on (for the most part). When I opened up the Parsl docs, I was originally turned off by the fact that it seemed like I needed to move all my import statements into the PythonApp definitions. For better or for worse, I closed the tab and moved on. Of course, I came back later and learned that this "requirement" really depends on the context.

So, I can't really see how it'd be harmful, but I do think it could be considered annoying (particularly when it's not actually needed). The docs as-written right now make it seem like it's a hard-and-fast rule when that's not the case.

benclifford pushed a commit that referenced this issue Oct 24, 2023
Clarifies problems users found in the documentation

 error in parallel documentation #2915: Decorators are not needed for ParslPoolExecutor.map
 Reloading default HighThroughputExecutor after parsl.clear() results in KeyError: 'block_id' exception #2871 : Configs may not be re-used
 Fixes Docs: Further clarification on how serialization impacts where imports need to be #2918 : Explain when functions need imports
 Fixes Allowing for inputs and outputs to take immutable types #2925 : Explain mutable types are undesirable

Thanks, @Andrew-S-Rosen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants