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

Make naming conventions consistent #109

Closed
annawoodard opened this Issue Mar 7, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@annawoodard
Copy link
Collaborator

annawoodard commented Mar 7, 2018

Since conventions are arbitrary, I do not have strong opinions about which are best; I think we should pick whatever is most common in the community and be consistent. We use a mix of naming styles. Consider the DFK constructor:

def __init__(self, config=None, executors=None, lazy_fail=True, appCache=True, rundir=None, fail_retries=2, checkpointFiles=None):

This mixes lowercase (rundir), lowercase with underscores (lazy_fail, fail_retries), and mixed case (appCache, checkpointFiles). I've seen a few cases of users making mistakes because they mixed up naming styles, so I think that even though it would require some existing user workflows to be updated, it might be worth it to move to a consistent style (and do so ASAP to minimize disruption). PEP-8 discourages mixedCase, so I vote we stick to: ClassName, ExceptionName, GLOBAL_CONSTANT_NAME, and lowercase_with_underscores for everything else.

I vaguely remember discussing this and I think we're already all in agreement, but I wanted to open an issue to keep track of this.

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented Mar 7, 2018

I'll wait for feedback; if there are objections to making this change, I'll just close the issue, otherwise, I can implement this.

@annawoodard annawoodard self-assigned this Mar 7, 2018

@danielskatz

This comment has been minimized.

Copy link
Collaborator

danielskatz commented Mar 7, 2018

I think this is a great idea, and we have talked about it before - the issue is that it may break backward compatibility, so we need to plan it carefully.

maybe we can talk about this next week while I am there?

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented Mar 7, 2018

Sounds good!

@kylechard

This comment has been minimized.

Copy link
Collaborator

kylechard commented Mar 7, 2018

Agreed on trying to be consistent and I also favor your approach " ClassName, ExceptionName, GLOBAL_CONSTANT_NAME, and lowercase with underscores for everything else"

@annawoodard annawoodard added this to the Parsl-0.6.0 milestone Apr 23, 2018

@yadudoc

This comment has been minimized.

Copy link
Member

yadudoc commented Jul 3, 2018

@annawoodard Can we close this one ?

@yadudoc yadudoc closed this Jul 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.