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.

Sign up for GitHub

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

Require Julia v1.10 (LTS) #140

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Require Julia v1.10 (LTS) #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c34557
Require Julia v1.10 (LTS)
dlfivefifty Dec 17, 2024
98fdfb3
Remove old code
dlfivefifty Dec 17, 2024
dd03f6a
Update .github/workflows/CI.yml
dlfivefifty Dec 18, 2024
ca20c4f
Update .github/workflows/CI.yml
dlfivefifty Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/CI.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ jobs:
fail-fast: false
matrix:
version:
- '1.0'
- 'min'
- 'lts'
dlfivefifty marked this conversation as resolved.
Show resolved Hide resolved
- '1'
- 'nightly'
os:
- ubuntu-latest
- macOS-latest
Expand Down
10 changes: 5 additions & 5 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "AbstractFFTs"
uuid = "621f4979-c628-5d54-868e-fcf4e3e8185c"
version = "1.5.0"
version = "1.6.0-dev"

[deps]
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
Expand All @@ -20,11 +20,11 @@ Aqua = "0.8"
ChainRulesCore = "1"
ChainRulesTestUtils = "1"
FiniteDifferences = "0.12"
LinearAlgebra = "<0.0.1, 1"
Random = "<0.0.1, 1"
Test = "<0.0.1, 1"
LinearAlgebra = "1"
Random = "1"
Test = "1"
Comment on lines +23 to +25
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure about these changes? A while ago I was told that "<0.0.1, 1" would be needed in some scenarios even on somewhat recent Julia versions. But I don't recall the details.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe related: Adding compat entry to stdlib breaks tests on Julia 1.3 - General Usage - Julia Programming Language

Copy link
Member

Choose a reason for hiding this comment

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

I think it was https://discourse.julialang.org/t/psa-compat-requirements-in-the-general-registry-are-changing/104958#update-november-9th-2023-2 that I had in mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

That discussion seems to be relevant only to Julia <v1.10.

Copy link
Member

Choose a reason for hiding this comment

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

The issue linked in the latest suggestion (of adding <0.0.1) seems to be present in Julia > 1.4 as well: https://discourse.julialang.org/t/why-random-jl-is-fixed-to-version-0-0-0/105957/2

Copy link
Member Author

Choose a reason for hiding this comment

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

But do we really want to support this setup when we aren't testing it?

Copy link
Member Author

@dlfivefifty dlfivefifty Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also in that discourse someone said:

Right, and the issue is only seen on older versions of Julia

Copy link
Member

@devmotion devmotion Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To my knowledge that's only correct if you do not perform any Pkg operations in your tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

But again, we haven't tested such a configuration and so it doesn't make sense to me why we would want to support it

Unitful = "1"
julia = "^1.0"
julia = "1.10"

[extras]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
Expand Down
5 changes: 0 additions & 5 deletions src/AbstractFFTs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,4 @@ export fft, ifft, bfft, fft!, ifft!, bfft!,
include("definitions.jl")
include("TestUtils.jl")

if !isdefined(Base, :get_extension)
include("../ext/AbstractFFTsChainRulesCoreExt.jl")
include("../ext/AbstractFFTsTestExt.jl")
end

end # module
2 changes: 1 addition & 1 deletion test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ end
# type inference checks of `rrule` fail on old Julia versions
# for higher-dimensional arrays:
# https://github.com/JuliaMath/AbstractFFTs.jl/pull/58#issuecomment-916530016
check_inferred = ndims(x) < 3 || VERSION >= v"1.6"
check_inferred = ndims(x) < 3
dlfivefifty marked this conversation as resolved.
Show resolved Hide resolved

test_frule(AbstractFFTs.fftshift, x, dims)
test_rrule(AbstractFFTs.fftshift, x, dims; check_inferred=check_inferred)
Expand Down
Loading