pyControl / code

pyControl GUI and framework code
https://pycontrol.readthedocs.io
GNU General Public License v3.0
21 stars 20 forks source link

Code formatter #82

Closed ThomasAkam closed 1 year ago

ThomasAkam commented 1 year ago

@alustig3 proposes to use the Black code formatter to standardise the formatting of all pyControl code going forward. I can see the advantages of standardised formatting but think that for user example code the Black defaults might result code that is less readable and accessable for users, see e.g. what Black does to the example hardware defintion:

# This hardware definition specifies that 3 pokes are plugged into ports 1-3 and a speaker into
# port 4 of breakout board version 1.2.  The houselight is plugged into the center pokes solenoid socket.

from devices import *

board = Breakout_1_2()

# Instantiate Devices.
left_poke = Poke(board.port_1, rising_event="left_poke", falling_event="left_poke_out")
center_poke = Poke(
    board.port_2, rising_event="center_poke", falling_event="center_poke_out"
)
right_poke = Poke(
    board.port_3, rising_event="right_poke", falling_event="right_poke_out"
)

speaker = Audio_board(board.port_4)

# Aliases
houselight = center_poke.SOL

as compared to the original:

# This hardware definition specifies that 3 pokes are plugged into ports 1-3 and a speaker into
# port 4 of breakout board version 1.2.  The houselight is plugged into the center pokes solenoid socket.

from devices import *

board = Breakout_1_2()

# Instantiate Devices.
left_poke   = Poke(board.port_1, rising_event = 'left_poke'  , falling_event = 'left_poke_out' )
center_poke = Poke(board.port_2, rising_event = 'center_poke', falling_event = 'center_poke_out')
right_poke  = Poke(board.port_3, rising_event = 'right_poke' , falling_event = 'right_poke_out')

speaker = Audio_board(board.port_4)

# Aliases
houselight = center_poke.SOL 

@alustig3 what are your thoughts on this and how would you feel about excluding the tasks and hardware defintions from the Black formatting?

ThomasAkam commented 1 year ago

After reading a bit more about how Black works I got it to format the example hardware definition better by adding a comma to the end of the arguments lists for the devices to get them split onto multiple lines:

# This hardware definition specifies that 3 pokes are plugged into ports 1-3 and a speaker into
# port 4 of breakout board version 1.2.  The houselight is plugged into the center pokes solenoid socket.

from devices import *

board = Breakout_1_2()

# Instantiate Devices.

left_poke = Poke(
    board.port_1,
    rising_event="left_poke",
    falling_event="left_poke_out",
)

center_poke = Poke(
    board.port_2,
    rising_event="center_poke",
    falling_event="center_poke_out",
)

right_poke = Poke(
    board.port_3,
    rising_event="right_poke",
    falling_event="right_poke_out",
)

speaker = Audio_board(board.port_4)

# Aliases
houselight = center_poke.SOL

I think this is as readable as the original, and using Black for everything would clearly be simpler, but I guess it will take a little bit of editing to ensure all files look good with the Black format.

alustig3 commented 1 year ago

@ThomasAkam I agree Black's default max line length of 88 is too small. Personally I think 120 is a good.

For my setup I am using Black formatter as well as Ruff linter for pointing out errors in the code.

in the root of the pyControl directory I have a pyproject.toml file with the following:

[tool.ruff]
line-length = 120

[tool.black]
line-length = 120

I think this gives much more reasonable results. This is the diff I get when running Black with a line-length of 120 on the same hardware definition file: CleanShot 2023-05-11 at 14 21 46

how would you feel about excluding the tasks and hardware defintions from the Black formatting?

I think it would be best to avoid excluding files, and instead adjust the max line-length using a pyproject.toml file. My suggestion would be 120, but I don't have strong feelings about it.

alustig3 commented 1 year ago

@ThomasAkam oops sorry, didn't see your follow up comment about adding commas. That solution could work as well.

ThomasAkam commented 1 year ago

@alustig I'm happy either using the default line length of 88 or 120. I'll aim to get the new data format branch merged with the dev branch in the next few days and then we can do the black formatting. It is pretty much done apart from some small additional functionallity I want to add to the data import module, so it would be great if you can take a look and see what you think.

alustig3 commented 1 year ago

sounds good @ThomasAkam. my vote would be to use line length of 120.

I've taken an initial look at the new data format branch and mades some changes https://github.com/pyControl/code/pull/83

ThomasAkam commented 1 year ago

Code now Black formatted with max line length 120 as of 457c28e83c3bc2ffd0e777ea8561745aba2d9a29