Conversation
|
With the latest commit, the tests are passing locally and the documentation is building just fine (locally also). |
|
Btw, after the suggestion of @nhz2 , I could compare the time to print the first DataFrames.jl in v2 and v3. We had an improvement here! Version 2: julia> @time using DataFrames
0.338423 seconds (522.48 k allocations: 37.232 MiB, 10.71% gc time, 3.79% compilation time)
julia> df = DataFrame(A = 1:5, B = 1:5);
julia> @time show(df)
5×2 DataFrame
Row │ A B
│ Int64 Int64
─────┼──────────────
1 │ 1 1
2 │ 2 2
3 │ 3 3
4 │ 4 4
5 │ 5 5 0.170112 seconds (651.53 k allocations: 35.599 MiB, 6.81% gc time, 99.55% compilation time: 40% of which was recompilation)Version 3: julia> @time using DataFrames
0.354778 seconds (556.40 k allocations: 39.779 MiB, 8.14% gc time, 3.70% compilation time)
julia> df = DataFrame(A = 1:5, B = 1:5);
julia> @time show(df)
5×2 DataFrame
Row │ A B
│ Int64 Int64
─────┼──────────────
1 │ 1 1
2 │ 2 2
3 │ 3 3
4 │ 4 4
5 │ 5 5 0.116783 seconds (288.77 k allocations: 17.764 MiB, 13.69% gc time, 99.56% compilation time) |
|
Thank you.
|
|
Hi @bkamins !
OK!
Done!
The issue arises because PrettyTables.jl no longer supports Julia 1.6. To minimize the maintenance burden, I’ve decided to always support the current version and the LTS version. Hence, for the 1.6 tests, the system is using PrettyTables v2, which results in test failures. Is this a problem? |
|
Regarding failing tests on 1.6. Thank you for the insight.
The reason is that otherwise we will have a constant problem that tests in 1.6 fail and it will be more difficult to detect the reason. Would any of these solutions work with you? Thank you! |
|
Hi @bkamins ! Ok! Here’s what I did: I copied the entire file for PrettyTables.jl v2 and added a flag to select which version to include in the tests. For |
|
Just for curiosity, why DataFrames.jl is still supporting Julia 1.6? It will not be updated anymore and we are not testing against 1.10, which is what people would use for LTS? |
|
@bkamins Done! Test are now passing in Julia 1.6. |
|
@ronisbr - thank you for working on this. I will be back to work on Monday and review the PR. Regarding 1.6 support - it was a decision made a year ago with @nalimilan as a precaution. @nalimilan - maybe indeed we can change to 1.10 now? |
|
Ah, this is indeed a difficult decision. I tend to favor keeping support for old versions as usually the burden isn't that high. But in the current case the fact that PrettyTables doesn't support 1.6 indeed leads to a lot of code duplication in DataFrames (and we've never supported two versions of a dependency in the past). Maybe we can require 1.10 now as it's already relatively old. |
Awesome!
So, if I understood correctly, I need to retain support for both versions 2 and 3 within the code during the transition of the Julia ecosystem to version 3. However, I should remove all the duplicated tests that were specifically designed for Julia 1.6, correct? |
|
OK. So let us do the following:
Later I will do a separate PR, that will clean up code base of technological debt we keep to have 1.6 support. @ronisbr - would that work with you? Thank you! |
Perfect! I will do that! |
|
Better not add a compat bound if it's not really necessary to use DataFrames, as some packages do not yet support CategoricalArrays 1.0, which means some users won't get the new DataFrames release. Anyway the compat bound shouldn't be needed to get the latest version for tests. |
|
@nalimilan CategoricalArrays v1.0 was launched on August 1st. Is there any problem using "0.10.0, 1" as compat bounds? Otherwise, it is obtaining v0.10.8 and the docs are failing. |
|
Btw, there is some really strange. DataFrames requires CategoricalArrays 0.10. However, the documentation build processing is using 1.0.1 (https://github.com/JuliaData/DataFrames.jl/actions/runs/17277010295/job/49036139753#step:5:58) I think it should not be possible. In this PR, it is obtaining the expected version (v0.10.8). That's why the latest commit should fix the build process. |
|
Ah OK, I always forget that CategoricalArrays is only a test dependency for DataFrames. So requiring version 1 should be OK, it shouldn't affect users. |
|
|
||
| # printed height always matches desired height, above a reasonable minimum | ||
| for a in 1:50, b in 1:50, h in 15:40 | ||
| for a in 1:50, b in 1:50, h in 17:40 |
There was a problem hiding this comment.
why do we start with 17 and not 15 here and below?
There was a problem hiding this comment.
In PrettyTables.jl v2, I saw a bug where we would print a table without the line separating the header and the data. In v3, I decided to always print at least the column label (including the separation line). Hence, in this case, we have 2 more lines to display (two separation lines in two grouped data frames).
There was a problem hiding this comment.
Is it Ok or is it a problem? The difference is that we now have:
GroupedDataFrame with 2 groups based on key: x
First Group (10 rows): x = 1
Row │ x
│ Int64
─────┼───────
⋮ │ ⋮
10 rows omitted
⋮
Last Group (10 rows): x = 2
Row │ x
│ Int64
─────┼───────
⋮ │ ⋮
10 rows omitted
instead of:
GroupedDataFrame with 2 groups based on key: x
First Group (10 rows): x = 1
Row │ x
│ Int64
⋮ │ ⋮
10 rows omitted
⋮
Last Group (10 rows): x = 2
Row │ x
│ Int64
⋮ │ ⋮
10 rows omitted
The previous version was confusing because you do not know if you have more header lines or not.
|
Hi @bkamins ! I fixed all your suggestions except for that about the number of lines. |
|
@nalimilan - looks good to me. Can you also please have a look at this PR before merging? codecov problems are due to PrettyTables.jl v2 support that is not invoked. Thank you! |
|
Sorry for the misunderstanding, but if we keep supporting PrettyTables 2, couldn't we keep supporting Julia 1.6? One big advantage of this is that we would continue testing the compat code. Having so much code that isn't tested at all isn't great. |
|
Jumping in because this PR is blocking various updates downstream...
My understanding is that everyone already moved to LTS, and that after this PR is merged @bkamins will clean up the source code further to remove uncovered lines. |
|
You could try using https://github.com/julia-actions/julia-downgrade-compat to test that. |
IMHO supporting Julia 1.6 is not good. In this case, we are testing against a version probably very few people are using and not testing against what a lot of people are using (LTS). Adding code to support both PrettyTables.jl v2 and v3 for now is important because, as @nhz2 mentioned, it will take sometime for people to upgrade their packages. In this case, we can avoid some problems for the community. |
|
Ah! Btw, the only code that will not be tested in this version are two functions to display DataFrames (in text and html). Notice that those functions have remaining almost unchanged since the initial PR to add support for PrettyTables.jl. EDIT: By the way, I am not against only supporting PrettyTables.jl v3! At least, it will make people upgrade faster :D |
|
Only supporting v3 is fine in theory, but in practice, it will likely create weird issues where people end up using super old versions of packages due to how Pkg.jl decides to resolve compat conflicts. For example, if someone wants to install the latest CUDA.jl and DataFrames.jl, Pkg.jl might decide to resolve the conflict by installing the latest DataFrames.jl, and then CUDA.jl from 2023 before it added PrettyTables.jl as a dep. |
Makes sense! In this case, IMHO, the way to go is what we have currently in this PR: supporting PrettyTables.jl v2 for those cases and drop support for Julia 1.6. |
|
@nalimilan BTW CategoricalArrays.jl specifies Julia v1.6, but relies on package extensions (weakdeps) in its Project.toml, which are only available in Julia v1.9 and later versions. You might think it supports Julia v1.6, but it shouldn't. |
Yes - this is what I had in mind. The coverage failure - as @ronisbr mentions - is in the "old" code that was essentially untouched. At some point we will remove it (and this is the reason why I asked @ronisbr to add a comment about removal of v2 support in the future). |
|
Fine with me. I'm not super happy to have untested code, but I guess that's OK since it's old code and we'll remove it at some point.
@juliohm CategoricalArrays uses Requires.jl on Julia 1.6. It's tested by CI so I guess it works. I think it was important to release 1.0 on Julia 1.6 as otherwise a package that would want to support 1.6 for some reason couldn't move to that major version easily. |
|
Hi @nalimilan !
Awesome! Yes, we will probably remove it very quickly! @bkamins maybe we are ready to merge? My next step will be to close the issues and add the LaTeX backend. |
|
We are ready to merge from my perspective. But let me wait for @nalimilan to approve the PR, as this is a major change, so I prefer to wait for it rather to rush with merging. Thank you for working on it! |
|
@ronisbr - thank you for a massive and excellent work. Now - as discussed let us work on open things related to display. If you feel that something is not doable or too hard to do please comment in the open issues and I will close them (or re-scope). Thank you! |
Awesome! Yes, I will do that! Thanks @bkamins |
|
Could you please tag a new release to propagate PrettyTables.jl v3?
Em qui., 4 de set. de 2025, 17:37, Ronan Arraes Jardim Chagas <
***@***.***> escreveu:
… *ronisbr* left a comment (JuliaData/DataFrames.jl#3510)
<#3510 (comment)>
@ronisbr <https://github.com/ronisbr> - thank you for a massive and
excellent work. Now - as discussed let us work on open things related to
display. If you feel that something is not doable or too hard to do please
comment in the open issues and I will close them (or re-scope). Thank you!
Awesome! Yes, I will do that! Thanks @bkamins <https://github.com/bkamins>
—
Reply to this email directly, view it on GitHub
<#3510 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZQW3KVGO6MMUWNNLZPR6T3RCPKBAVCNFSM6AAAAACE6YRCM2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENJVGU2TQNRXHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
We will make this release soon. |
5 participants
The PR updates the support in DataFrames.jl to PrettyTables.jl v3. I’ve also updated the API call and the tests. As we can see from the tests, the output changes are minor. For the user, the printed version of DataFrames.jl should look exactly the same except in very edge cases.
@bkamins I reviewed the list of open display-related issues, and I can address some of them. However, this migration is currently blocking some internal workflows because we can’t use DataFrames with PrettyTables v3 simultaneously. Could we please merge this support, and then I’ll handle the remaining issues?