Skip to content

Conversation

@YingboMa
Copy link
Member

It's SciML/Sundials.jl#357 with

normalize_line_endings = "unix"

@YingboMa
Copy link
Member Author

@YingboMa YingboMa force-pushed the myb/format branch 3 times, most recently from da0d879 to b355047 Compare May 23, 2022 04:07
@YingboMa YingboMa requested a review from ChrisRackauckas May 23, 2022 04:07
aeqs = algeqs(state.structure)
var_eq_matching = Matching{Union{Unassigned, SelectedState}}(tear_graph_modia(state.structure;
varfilter=var->var in algvars, eqfilter=eq->eq in aeqs))
varfilter = var -> var in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These few lines seem a bit awkward. This could be a good-looking resolution:

    var_eq_matching = Matching{Union{Unassigned, SelectedState}}(
        tear_graph_modia(state.structure; varfilter = var -> var in algvars, eqfilter = eq -> eq in aeqs))

But this goes against the idea that function arguments should be aligned with the opening bracked. It might be helpful to allow arguments on new lines, even if just for cases like this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easier to just write two lines in this case.

ps::Vector
"""Array variables."""
var_to_name
var_to_name::Any
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it doesn't matter, but would it make more sense for this to be Dict{Symbol,Any} rather than Any as the formatter is selecting?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to have untyped fields to be any, but yes, in this case, it should be Dict{Symbol,Any}

@YingboMa
Copy link
Member Author

Thanks everyone for reviewing! Seems like there are only a few edge cases left that can be resolved by manual formatting. Let's track this in an issue.

@YingboMa YingboMa merged commit fa5de28 into master May 24, 2022
@YingboMa YingboMa deleted the myb/format branch May 24, 2022 17:12
@YingboMa YingboMa mentioned this pull request May 27, 2022
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

Successfully merging this pull request may close these issues.

6 participants