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

black config - 120 chars? #118

Closed
albertz opened this issue Feb 18, 2023 · 9 comments
Closed

black config - 120 chars? #118

albertz opened this issue Feb 18, 2023 · 9 comments

Comments

@albertz
Copy link
Member

albertz commented Feb 18, 2023

I would propose the following pyproject.toml config:

[tool.black]
line-length = 120
target-version = ["py37"]

Currently black uses the default 88 chars. So this would change it to 120 chars. I personally would prefer that. Any objections?
I'm not so sure about the Python target-version but I guess Python >=3.7 is ok?

albertz added a commit that referenced this issue Feb 18, 2023
But maybe we should change it? #118
@JackTemaki
Copy link
Contributor

I also prefer 120. Target version is ok.

@albertz
Copy link
Member Author

albertz commented Feb 18, 2023

Corresponding issue in i6_core: rwth-i6/i6_core#362

@albertz
Copy link
Member Author

albertz commented Feb 23, 2023

So, it is decided then? Or what exactly do we wait for now?

@JackTemaki
Copy link
Contributor

we wait for any response in rwth-i6/i6_core#362

@albertz
Copy link
Member Author

albertz commented Feb 23, 2023

Why? Because you want it consistent to that? We can also decide independently here, or not?

@christophmluscher
Copy link
Contributor

I would keep the decision consistent but the timing independent between core and experiments.

Since I have a number of PR on common/rasr, if possible, I would like to to the black with 120 on those files.

@albertz
Copy link
Member Author

albertz commented Feb 23, 2023

I mean, we can also argue that it should be consistent to RETURNN-common, or RETURNN.

But it anyway seems like most people vote for 120.

@christophmluscher
Copy link
Contributor

IMHO core and experiments are both Sisyphus recipes. Returnn and returnn common are not.
I'd rather argue Sisyphus should have the same black rules than returnn

@albertz
Copy link
Member Author

albertz commented Feb 28, 2023

Ok, @curufinwe agreed to 120 chars in i6_core (rwth-i6/i6_core#362). I guess it is decided then that we use 120 chars both here in i6_exp and in i6_core.

I will just apply it directly here in i6_exp. Someone else can do it for i6_core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants