Discussion: Preferred code styles

[jrycw]

I'd like to gather opinions from the team regarding preferred code styles for certain situations:

1. return statements

Currently, we have three types of return styles in our codebase:

• return: Implicitly returns None.
• return None: Explicitly returns None.
• No return statement: Implicitly returns None.

I would propose adopting the last style consistently (not having return statement) .

2. Unpacking

Currently, we often perform unpacking operations like this:

 coord = [1, 2]

 x = coord[0] # x=1
 y = coord[1] # y=2

I suggest changing the code style to the following for conciseness and flexibility:

 coord = [1, 2]

 x, y = coord # x=1, y=2

This change makes the code more concise and allows for unpacking any data type that can be unpacked, such as an iterator:

coord = (i for i in range(1, 3))

x, y = coord # x=1, y=2

3. isinstance usage

In some places in the codebase, direct type comparisons like if type(x) == list are used. I propose changing these to if isinstance(x, list) for improved flexibility and robustness.

4. Avoiding indexing assumptions

While indexing is convenient, it's advantageous to avoid reliance on it when possible. For instance, rather than accessing the first element of a data structure with x[0], we can use next(iter(x)) if x is iterable (typically it is). This approach enhances code versatility and adaptability to diverse data structures.

While there's no definitive right or wrong when it comes to code style, consistency within our codebase is highly beneficial. If we can reach a consensus on the preferred code style, I'm more than willing to submit a PR reflecting our decision.

[machow]

Hey, thanks for taking the time to carefully write out the challenges and helpful suggestions! I think everything you've said for 1-3 is spot on, and would be uncontroversial and useful to refactor to use! (I'm surprised 3 is not picked up by the linter 😓 ).

RE 4, I know that codebases juggle a bit between functions that operate on lists/sequences of things, verses a single thing. I have less strong opinions about how a first element is grabbed. There might be worthwhile cases to clean up though, like when...

• a function expects a list of elements, but then
• everything that uses it wraps a single element in a list, then
• pulls the first element out of the result.

[jrycw]

I'll close this issue since the related discussion has been addressed in #305.