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

New formatting turns optional positional argument into keyword argument #878

Open
gdalle opened this issue Oct 16, 2024 · 8 comments · May be fixed by #883
Open

New formatting turns optional positional argument into keyword argument #878

gdalle opened this issue Oct 16, 2024 · 8 comments · May be fixed by #883

Comments

@gdalle
Copy link

gdalle commented Oct 16, 2024

Kudos on the full rewrite!
Unfortunately I have encountered a bug when I tried to apply the new formatting in JuliaDiff/DifferentiationInterface.jl#589. An unnamed, Val-typed positional argument was "formatted" into a keyword argument (see this comment).

Schematically, the following code

function f(x, y, ::Val{N}=Val(1)) where {N}

turned into (note the semicolon)

function f(x, y; (::Val{N})=Val(1)) where {N}

which changed the semantics.

EDIT: As discussed below, this apparently applied to all optional positional arguments, named or not. It might be due to BlueStyle.

@MilesCranmer
Copy link
Contributor

Same bug unfortunately. All optional arguments (named or not) seem to get converted to keyword arguments.

@domluna do you think you could yank versions 2.0.0 and 1.0.62 until these bugs and #879 are patched? (I am unfortunately needing to patch all my CI scripts at the moment).

Thanks for all your hard work!!

@domluna
Copy link
Owner

domluna commented Oct 16, 2024

@domluna do you think you could yank versions 2.0.0 and 1.0.62 until these bugs and #879 are patched? (I am unfortunately needing to patch all my CI scripts at the moment).

what do I need to do to yank the release?

@avik-pal
Copy link

@domluna
Copy link
Owner

domluna commented Oct 17, 2024

@MilesCranmer why does 1.0.62 need to be yanked?

@MilesCranmer
Copy link
Contributor

The latest version of 1 seemed to change the rules in a breaking way, not sure the number

@domluna
Copy link
Owner

domluna commented Oct 17, 2024

The latest version of 1 seemed to change the rules in a breaking way, not sure the number

there hasn't been any major changes for the past few releases, is there an example?

@gdalle gdalle changed the title New formatting turns positional into keyword argument New formatting turns optional positional argument into keyword argument Oct 17, 2024
@gdalle
Copy link
Author

gdalle commented Oct 17, 2024

Here's a minimum working example, where the first two formattings work and the last one introduces an unwanted kwarg:

julia> using JuliaFormatter  # version 2

julia> format_text("""
       function f(x::Vector{T}) where {T}
           return nothing
       end
       """, BlueStyle()) |> println
function f(x::Vector{T}) where {T}
    return nothing
end


julia> format_text("""
       function f(x::Vector=[])
           return nothing
       end
       """, BlueStyle()) |> println
function f(x::Vector=[])
    return nothing
end


julia> format_text("""
       function f(x::Vector{T}=[]) where {T}
           return nothing
       end
       """, BlueStyle()) |> println
function f(; x::Vector{T}=[]) where {T}
    return nothing
end

So it might be the combination of a parametric type and a default value that is to blame

@MilesCranmer
Copy link
Contributor

@domluna I see, then that’s probably my mistake, and it’s only 2.0.0 that should be yanked.

(What I assume happened is I added 1.0.62 to my env, but 2.0.0 was already loaded somewhere from my base env, and that’s what actually did the formatting. I’ll double check this though.)

abelsiqueira added a commit to TulipaEnergy/TulipaEnergyModel.jl that referenced this issue Oct 17, 2024
DilumAluthge pushed a commit to JuliaRegistries/General that referenced this issue Oct 17, 2024
abelsiqueira added a commit to TulipaEnergy/TulipaEnergyModel.jl that referenced this issue Oct 18, 2024
abelsiqueira added a commit to TulipaEnergy/TulipaEnergyModel.jl that referenced this issue Oct 18, 2024
datejada pushed a commit to TulipaEnergy/TulipaEnergyModel.jl that referenced this issue Oct 18, 2024
* Add debug step to Lint workflow

* Add JuliaFormatter at version 1 in the Lint workflow

Due to to domluna/JuliaFormatter.jl#878.

* Fix link exclusion in lychee

* Add codecov patch target (95%)
@domluna domluna linked a pull request Oct 19, 2024 that will close this issue
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 a pull request may close this issue.

4 participants