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

Fix on Julia 1.7 #33

Merged
merged 8 commits into from
Jan 21, 2022
Merged

Fix on Julia 1.7 #33

merged 8 commits into from
Jan 21, 2022

Conversation

oxinabox
Copy link
Member

Closes #32 and #28

Vararg on 1.7 is not a type any more.
So it gets its own path through the code.
This does mean we actually generate nicer code with that.
(because it was easier to generate nicer code than workout how to generate code identical to what we had before).
But that also means we needed to put some branches in the tests where they do produce nicer code

This PR blocks fixing Nabla to work on Julia 1.7.
(though old versions of Nabla that are PreChainRules are not blocked by this. Including the version in production)

src/method.jl Outdated Show resolved Hide resolved
src/method.jl Outdated Show resolved Hide resolved
src/method.jl Outdated Show resolved Hide resolved
test/method.jl Outdated Show resolved Hide resolved
test/method.jl Outdated Show resolved Hide resolved
test/method.jl Outdated Show resolved Hide resolved
test/method.jl Outdated Show resolved Hide resolved
test/method.jl Outdated Show resolved Hide resolved
test/type_utils.jl Outdated Show resolved Hide resolved
test/type_utils.jl Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #33 (8983be6) into master (090c74b) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   97.69%   97.77%   +0.08%     
==========================================
  Files           5        5              
  Lines         217      225       +8     
==========================================
+ Hits          212      220       +8     
  Misses          5        5              
Impacted Files Coverage Δ
src/method.jl 97.41% <100.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 090c74b...8983be6. Read the comment docs.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
test/type_utils.jl Outdated Show resolved Hide resolved
@oxinabox oxinabox mentioned this pull request Jan 18, 2022
test/type_utils.jl Outdated Show resolved Hide resolved
test/type_utils.jl Outdated Show resolved Hide resolved
test/type_utils.jl Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@iamed2
Copy link
Member

iamed2 commented Jan 19, 2022

I don't know if you want to handle this but:

In Julia 1.7, with this change:

julia> ExprTools.name_of_type(Vararg)
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] getproperty
   @ ./Base.jl:42 [inlined]
 [2] name_of_type(x::Core.TypeofVararg)
   @ ExprTools ~/.julia/packages/ExprTools/fES3C/src/method.jl:184
 [3] top-level scope
   @ REPL[5]:1

In Julia 1.6, with or without this change:

julia> ExprTools.name_of_type(Vararg)
:(Vararg{T, N} where {T, N})

@oxinabox
Copy link
Member Author

name_of_type isn't exactly part of the public API.
But I feel that should work so I will fix that

test/method.jl Outdated Show resolved Hide resolved
test/method.jl Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@oxinabox oxinabox merged commit 3fdc246 into master Jan 21, 2022
@oxinabox oxinabox deleted the ox/17 branch January 21, 2022 18:43
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.

CI failing in Julia 1.7
2 participants