Change indentation to 4 spaces?

[albertz]

It is 2 spaces for consistency to RETURNN. (RETURNN has 2 spaces for historical reasons.) However, i6_experiments and i6_core use 4 spaces.

Change this here as well? (Maybe also in RETURNN? But that would be maybe a separate discussion. -> https://github.com/rwth-i6/returnn/issues/1156)

[albertz]

@JackTemaki @christophmluscher @Atticus1806 I tagged you to get some opinion on this. Maybe also @curufinwe, @michelwi, @mmz33 ?

[christophmluscher]

My reasoning would be returnncommon from a code base point of view belongs to returnn and not to Sisyphus / i6*. IMHO code convention should follow returnn.

In general as a group we should maybe strive for common code formatting rules. But I don't think this is a must.

What do you see as the clear benefit from switching indentation?

[albertz]

What do you see as the clear benefit from switching indentation?

Consistency.

Now in my Sisyphus i6_core/i6_experiments/returnn_common based setup, I basically work on all these three repos. Not so much RETURNN itself anymore, so not sure if your initial statement is really correct.

But as said, maybe we also should change RETURNN itself, but that would be a somewhat separate discussion.

[albertz]

What do you see as the clear benefit from switching indentation?

Consistency.

To extend further on this:

Sometimes I even move code from i6_experiments over to returnn_common, or vice versa.

[christophmluscher]

My reasoning would be returnncommon from a code base point of view belongs to returnn and not to Sisyphus / i6*. IMHO code convention should follow returnn.

so do you agree with this??

Sometimes I even move code from i6_experiments over to returnn_common, or vice versa.

but from a practical point of view, it seems i6_experiments and returnn_common are closer together?

what would be the main disadvantage in your opinion switching indentation in returnn / returnn_common? for example we could just run black, so the transition should be fairly straightforward..

[albertz]

My reasoning would be returnncommon from a code base point of view belongs to returnn and not to Sisyphus / i6*. IMHO code convention should follow returnn.

so do you agree with this??

No. Not sure. They all belong kind of somewhat together (i6_core/i6_exp have lots of RETURNN specific stuff, but then of course also other unrelated stuff), while also being separate (although you could maybe argue whether returnn_common really needs to be separate from returnn). I don't think there is a clear yes or no answer to this.

Sometimes I even move code from i6_experiments over to returnn_common, or vice versa.

but from a practical point of view, it seems i6_experiments and returnn_common are closer together?

Yes, that's exactly what I mean. In i6_experiments.users, I define models, or model building blocks. In returnn_common, I do the same. So I imagine I would sometimes play around with sth new, and have that first in i6_exp, and when this becomes useful for others, we would move it over to returnn_common.

Or sometimes I would want to take an existing building block of returnn_common but play around with it, test some variations. Then I would copy it over to i6_exp.

So there is quite some overlap between returnn_common and i6_exp, while there is not really any overlap between returnn_common and returnn.

what would be the main disadvantage in your opinion switching indentation in returnn / returnn_common? for example we could just run black, so the transition should be fairly straightforward..

Black is another separate aspect. Of course related, but just changing indentation could be a first step. I'm not really sure on Black, so I would keep that for a separate discussion. I'm not really such a big fan of Black...

[christophmluscher]

Theoretically, returnn_common builds on returnn, so should probably use the same code formatting conventions. Practice-wise I agree we should probably make the movement of code between i6_experiments and returnn_common as easy as possible.

So there is quite some overlap between returnn_common and i6_exp, while there is not really any overlap between returnn_common and returnn.

I would then argue for the practical approach, because we have less work later, aka 4-indent.

[JackTemaki]

+1 for 4-space indent