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

code layout improvements #2536

Merged
merged 2 commits into from
Nov 13, 2020
Merged

code layout improvements #2536

merged 2 commits into from
Nov 13, 2020

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Nov 13, 2020

I wanted to fix this problems in error printing (missing space in both cases):

julia> df = DataFrame(a=1)
1×1 DataFrame
 Row │ a     
     │ Int64 
─────┼───────
   1 │     1

julia> push!(df, (a=1, b=2))
ERROR: ArgumentError: `push!` with `cols` equal to `:setequal`requires `row` to have the same number of elements as the number of columns in `df`.

julia> dfv = @view df[:, :]
1×1 SubDataFrame
 Row │ a     
     │ Int64 
─────┼───────
   1 │     1

julia> dfv.x = [1]
ERROR: ArgumentError: Replacing or adding of columns of a SubDataFrame is not allowed.Instead use `df[:, col_ind] = v` or `df[:, col_ind] .= v` to perform an in-place assignment.

So I also reviewed the whole source code to make sure that we always use a space at the end of the line when we break it.

  • I changed very long lines in isapprox.

This PR does not change anything from the functionality perspective.

@bkamins bkamins added the non-breaking The proposed change is not breaking label Nov 13, 2020
@bkamins bkamins added this to the 1.0 milestone Nov 13, 2020
@bkamins bkamins mentioned this pull request Nov 13, 2020
20 tasks
@bkamins
Copy link
Member Author

bkamins commented Nov 13, 2020

@nalimilan - only coverage fails.

src/other/index.jl Outdated Show resolved Hide resolved
@bkamins bkamins merged commit c0be8b1 into JuliaData:master Nov 13, 2020
@bkamins bkamins deleted the code_layout branch November 13, 2020 15:58
@bkamins
Copy link
Member Author

bkamins commented Nov 13, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-breaking The proposed change is not breaking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants