Add .JuliaFormatter.toml for contributers

[jacobusmmsmit]

This pull request adds a configuration file to be used with JuliaFormatter.jl, the built-in formatter of Julia for VSCode and the most popular one in general.

This will allow a consistent style to be enforced throughout the repository and make the code more similar to related packages in the Julia ecosystem.

[Datseris]

oof I don't like this formatting style at all... I like only 1 normal additional identitation level when having open functions such as

name(
    arg1, arg2;
    kwarg1, kwarg2
)

I guess I would need to study the julia formatter docs to see how to achieve this.

[Datseris]

Also, let's put the format check CI here as well, as shown here: https://github.com/SciML/SciMLStyle#juliaformatter

[jacobusmmsmit]

I added the file to CI and changed its triggers to be the same as the current ci.yml. I wasn't sure where to fit it into that file, or whether it even belonged in there, so I made a new one.

[Datseris]

I've read the Jula formatter but I don't know which option corresponds to this: #723 (comment) :(

if you know please let me know?

[jacobusmmsmit]

Honestly, I don't know myself. Might be best to ask the JuliaFormatter team themselves

[jacobusmmsmit]

Ok so Blue style is closer to what you prefer, from what I can tell. It's also my preferred one over SciML by far. What do you think?

[jacobusmmsmit]

Btw, in the case of hard formatting, maybe it's best to just enforce it in src, often in examples and docs it's nice to be able to go against strict guidelines, but when 10 people are contributing to a project automatically enforcing a style is nice.

[codecov-commenter]

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (37d3a13) 92.27% compared to head (a4b302c) 88.58%.

❗ Current head a4b302c differs from pull request most recent head 3f5d6d5. Consider uploading reports for the commit 3f5d6d5 to get more accurate results

Files	Patch %	Lines
src/spaces/graph.jl	40.00%	9 Missing ⚠️
src/Agents.jl	66.66%	3 Missing ⚠️
src/deprecations.jl	0.00%	3 Missing ⚠️
src/spaces/grid_multi.jl	89.28%	3 Missing ⚠️
src/spaces/walk.jl	86.66%	2 Missing ⚠️
src/core/model_abstract.jl	75.00%	1 Missing ⚠️
src/simulations/sample.jl	66.66%	1 Missing ⚠️
src/submodules/pathfinding/astar.jl	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #723      +/-   ##
==========================================
- Coverage   92.27%   88.58%   -3.69%     
==========================================
  Files          33       30       -3     
  Lines        2277     1963     -314     
==========================================
- Hits         2101     1739     -362     
- Misses        176      224      +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Datseris]

oh yeah we really have to do finish this but I haven't had the time yet. I don't like current style at all though. I don't like that it enforces a line break after every single argument, I find it super pointless...

[jacobusmmsmit]

Ok, so this new style I made by altering MinimalStyle is pretty... well, minimal in the changes that it makes. The biggest thing is that because margin = 10_000, there will never be any changes to how arguments are handled. I'm sure there's a more elegant solution to this, but it does, at least, allow us to enforce things like:

• Always explicitly returning with return,
• Consistent use of whitespace,
• Trailing commas in iterables,
• Removing unnecessary newlines,
• Adding newlines to the end of files,

Personally, I'd prefer if whitespace_in_typedefs were false as our type definitions are quite long and benefit from being compacted. My most recent commit does this.

Finally, before this is merged (if it is), there might be a better way to describe the formatting than what we have i.e. if we can specify style = "minimal" with only the changes we want to this, rather than describing "minimal" and our additional changes.

[Datseris]

Yeah this is where we have a problem:

that's the one thing that really bothers me that the function has a closing parenthesis at 0 identation. There is no way/option you are aware of that makes it so that the function arguments and closing parenthesis are idented one more time?

[Datseris]

? Finally, before this is merged (if it is), there might be a better way to describe the formatting than what we have i.e. if we can specify style = "minimal" with only the changes we want to this, rather than describing "minimal" and our additional changes.

This will become a style for JuliaDyunamics which I'll apply to all repos. So I'll do a PR to the Julia formatter package to add this style.

[jacobusmmsmit]

There is no way/option you are aware of that makes it so that the function arguments and closing parenthesis are idented one more time?

I'm not 100% sure if this is what you mean but check out YAS style guide which is inspired by the one used in JuMP. Specifically, see here, is this what you're after?

After looking at my preferred compared to YAS style, they are actually very close: YAS is slightly more strict as it additionally imposes

1. A margin limit (which makes the biggest difference)
2. Replace x |> f with f(x) (fair enough, we're not doing any data analysis here so pipes are rare)
3. Replace import with using (only when interchangeable afaik)
4. Annotate untyped struct fields with Any (also fair enough, we shouldn't have untyped fields in structs anyway)

If you want to try out YAS, cd into Agents.jl on main or some branch and run

using JuliaFormatter; format("src/.", YASStyle())

[Datseris]

Grrrrr this is so annoying. here is what I want:

funccall(
    arg1,
    arg2,
    arg3,
)
    x = 2 # operations
end

to instead be

funccall(
        a, b, c, # same row unless they exceed margin
        d, e, g, # notice that this is twice idented, one more from original
    )
    x = 2 # operations
end

but I have no idea how to achieve this based on the current documentation of the formatter. These are the only changes that I care about implementing. For everything else I'm happy to follow the blue style.

[jacobusmmsmit]

Wait, this is exactly what SciML recommends... I wonder what doesn't work here.

[Datseris]

Yes but SciML indents on the function opening parenthesis, which is bad if functions have long names. I want to ident one level more than the funtion's natural identation :D

Maybe I am asking for too much and we should just go with blue style... :(

[jacobusmmsmit]

Wait but Blue says to do exactly that (scroll down a bit), it just treats kwargs differently. Maybe Blue is the one true style?

[Datseris]

I'm not sure blue style is correct. Here is what I want:

function f(
        a, b, c, # same row unless they exceed margin
        d, e, g, # notice that this is twice idented, one more from original
    )
    x = 2 # operations
end

here is what Blue style does, at least when used with JuliaFormatter.jl:

function f(
    a,
    b,
    c,
    d,
    e,
    f,
)
    x = 2 # operations
end

so there are two differnces: first that I don't want a line break after every single argument, only when they exceed the 92 chars, and second that I prefer a second level of identation when writing the function arguments, so that the function closing parenthesis has 1 level of identation, just like the function code.

I can live with second thing being unsatisfied. I can live with this:

function f(
    a, b, c, # same row until 92 chars
    e, f, g,
)
    x = 2
end

but what I cannot live with, 100%, is enforcing a line break after every single function argument.

[Datseris]

with everything else in the blue style I am completely fine with btw. Let's just try to see if we can address this issue and if not, we give up and go with blue style... :(

[Tortar]

I like not to have line breaks after each argument too, fortunately if I read correctly actually the SciML format should be okay?

From the docs:

If the number of arguments is too large to fit into a 92 character line, then use as many arguments as possible within a line and start each new row with the same indentation, preferably at the same column as the ( but this can be moved left if the function name is very long.

[jacobusmmsmit]

The example given to this point:

# Yes
function my_large_function(argument1, argument2,
                           argument3, argument4,
                           argument5, x, y, z)

# No
function my_large_function(argument1,
                           argument2,
                           argument3,
                           argument4,
                           argument5,
                           x,
                           y,
                           z)

This is exactly what @Datseris wrote in his example above.

[Tortar]

yes the only difference is that this:

function my_large_function(
    argument1, argument2, argument3, 
    argument4, argument5, x, y, z)
    s = 3
end

in SciMLStyle will be converted to

function my_large_function(argument1, argument2, argument3,
    argument4, argument5, x, y, z)
    s = 3
end

but even these sintaxes are allowed:

function my_large_function(argument1, 
    argument2, argument3, argument4, 
    argument5, x, y, z)
    s = 3
end

function my_large_function(argument1, 
    argument2, argument3, 
    argument4, argument5, 
    x, y, z)
    s = 3
end

the difference is then that it wants to put at least one argument at the name of the function level. it doesn't sound bad to me.

But It isn't very enforcing since one can choose one or the other way and it will be allowed. Maybe this is a weak point though

[Tortar]

Last option: make a new function definition setting an option :D

Maybe it is not that hard to create a new setting like this in JuliaFormatter, who knows

[Datseris]

I think this is the option to consider. If not possible, we just go with:

function my_large_function(argument1, argument2, argument3,
    argument4, argument5, x, y, z)
    s = 3
end

which is what the SCimlStyle will do (I think).

p.s. notice that this PR needs to be re0opened due to the huge amount of code changes that have happened in the meantime.

[Tortar]

Actually 3 weeks ago the SciML style changed enforcing the double-indent style so now it is usable as it is! See domluna/JuliaFormatter.jl#776

[jacobusmmsmit]

p.s. notice that this PR needs to be re0opened due to the huge amount of code changes that have happened in the meantime.

I think we should keep it on the same PR for discoverability, but yes throw away the changes, pull from main, and add a formatting merge commit.

[Datseris]

I think we should keep it on the same PR for discoverability, but yes throw away the changes, pull from main, and add a formatting merge commit.

ok, can you do that with the new SciML formatter?

[jacobusmmsmit]

Strangely, when I run JuliaFormatter.format() I get a parsing error in src/core/model_standard.jl. I'm using the latest version of JuliaFormatter 1.0.42, on Julia 1.9.4. The parsing error is reported to be on the last line.

Can anyone else try checking out main, adding the newest version of JuliaFormatter to their base env and running using JuliaFormatter; JuliaFormatter.format("src")?

Additionally precompiling I get the following error:

(Agents) pkg> precompile
Precompiling project...
  ✗ Agents
  0 dependencies successfully precompiled in 6 seconds. 104 already precompiled.

ERROR: The following 1 direct dependency failed to precompile:

Agents [46ada45e-f475-11e8-01d0-f70cc89e6671]

Failed to precompile Agents [46ada45e-f475-11e8-01d0-f70cc89e6671] to "/Users/smit/.julia/compiled/v1.9/Agents/jl_d8Uy3I".
ERROR: LoadError: MethodError: no method matching namify(::Nothing)

[Tortar]

Already tried...same error as you, it is probably a bug in the formatter...

Also, let's put the format check CI here as well, as shown here: https://github.com/SciML/SciMLStyle#juliaformatter

this makes me think that enforcing it on CI could be just too much struggle for contributors.

If you try JuliaFormatter.format(".") you'll find even more errors :D

[Tortar]

Judging from ERROR: LoadError: MethodError: no method matching namify(::Nothing) it probably messed up the @agent macro

[jacobusmmsmit]

That's a shame :/
I was hoping this would end the formatting saga and get us more in line with the SciML world. That said, code inspection did reveal a couple things I can quickly address.

[Tortar]

I was hoping this would end the formatting saga and get us more in line with the SciML world

I think that for this chapter of the saga we should just merge a formatting refactoring + a .juliaformatter.toml file and in the sequel add the CI work :D

[jacobusmmsmit]

Sure, but first JuliaFormatter.format() has to work :P, no use committing a toml if the command errors.

[Tortar]

Found the bug with model_standard.jl and opened an issue here domluna/JuliaFormatter.jl#779

It is on line 229:

elseif !(pos_type <: SVector{D,<:AbstractFloat} where {D} || (!isconcretetype(A) && pos_type <: SVector{D} where {D}))

to overcome use:

elseif !((pos_type <: SVector{D,<:AbstractFloat} where {D}) || (!isconcretetype(A) && pos_type <: SVector{D} where {D}))

[Tortar]

The other macro bug is probably domluna/JuliaFormatter.jl#669

[Datseris]

Thanks for the efforts! I'm trying to keep track of this. Where are we now? We need to make the change of #723 (comment) in the code. Regarding the @agent problems, can we just add one more clause that simply disables formatting macros? It would be a deviation from SciML but we don't care about that. We just need a JuliaDynamics-wide format anyways.

[jacobusmmsmit]

I don't think there's a "disable macro formatting" option, but it seems to be being worked on in some form. Perhaps the outcome will be that we disable formatting for all of our own macros via the options, that seems reasonable.

[domluna]

If you want more control over nesting, such as avoiding 1 argument per line I suggest turning on join_lines_based_on_source

https://domluna.github.io/JuliaFormatter.jl/dev/#join_lines_based_on_source