Agents.jl
Agents.jl copied to clipboard
Add .JuliaFormatter.toml for contributers
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.
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.
Also, let's put the format check CI here as well, as shown here: https://github.com/SciML/SciMLStyle#juliaformatter
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.
I've read the Jula formatter but I don't know which option corresponds to this: https://github.com/JuliaDynamics/Agents.jl/pull/723#issuecomment-1364590737 :(
if you know please let me know?
Honestly, I don't know myself. Might be best to ask the JuliaFormatter team themselves
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?
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 Report
Attention: 23 lines
in your changes are missing coverage. Please review.
Comparison is base (
37d3a13
) 92.27% compared to head (a4b302c
) 88.58%.
:exclamation: 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 :warning: |
src/Agents.jl | 66.66% | 3 Missing :warning: |
src/deprecations.jl | 0.00% | 3 Missing :warning: |
src/spaces/grid_multi.jl | 89.28% | 3 Missing :warning: |
src/spaces/walk.jl | 86.66% | 2 Missing :warning: |
src/core/model_abstract.jl | 75.00% | 1 Missing :warning: |
src/simulations/sample.jl | 66.66% | 1 Missing :warning: |
src/submodules/pathfinding/astar.jl | 80.00% | 1 Missing :warning: |
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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...
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.
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?
? 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.
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
- A margin limit (which makes the biggest difference)
- Replace
x |> f
withf(x)
(fair enough, we're not doing any data analysis here so pipes are rare) - Replace
import
withusing
(only when interchangeable afaik) - 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())
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.
Wait, this is exactly what SciML recommends... I wonder what doesn't work here.
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... :(
Wait but Blue says to do exactly that (scroll down a bit), it just treats kwargs differently. Maybe Blue is the one true style?
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.
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... :(
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.
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.
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
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
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.
Actually 3 weeks ago the SciML style changed enforcing the double-indent style so now it is usable as it is! See https://github.com/domluna/JuliaFormatter.jl/pull/776
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.
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?
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)
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
Judging from ERROR: LoadError: MethodError: no method matching namify(::Nothing)
it probably messed up the @agent
macro