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

Jump to bottom

Add get method, tests for Tuple and AbstractArrays :: Take 2 (#40809) #41007

Merged
merged 3 commits into from
May 31, 2021
Merged

Add get method, tests for Tuple and AbstractArrays :: Take 2 (#40809) #41007

merged 3 commits into from
May 31, 2021

Conversation

Pramodh-G
Copy link
Contributor

As specified in the PR with the same name (#40856 ), I had messed up my fork and had to start with a fresh fork. Sorry for this, I'm trying to improve :) .
I added tests that were mutating, but I couldn't add CartesianIndex(cc: @mcabbott ). I was getting this error while make ing julia. I would be glad to fix this if someone could guide me through it.

error during bootstrap:
LoadError(at "compiler/compiler.jl" line 3: LoadError(at "abstractarray.jl" line 1473: UndefVarError(var=:CartesianIndex)))
jl_undefined_var_error at /home/learnedfool/github_repos/julia/src/rtutils.c:132
jl_eval_global_var at /home/learnedfool/github_repos/julia/src/interpreter.c:141 [inlined]
eval_value at /home/learnedfool/github_repos/julia/src/interpreter.c:187
do_call at /home/learnedfool/github_repos/julia/src/interpreter.c:116
eval_value at /home/learnedfool/github_repos/julia/src/interpreter.c:206
eval_stmt_value at /home/learnedfool/github_repos/julia/src/interpreter.c:157 [inlined]
eval_body at /home/learnedfool/github_repos/julia/src/interpreter.c:575
jl_interpret_toplevel_thunk at /home/learnedfool/github_repos/julia/src/interpreter.c:718
top-level scope at abstractarray.jl:1473
jl_toplevel_eval_flex at /home/learnedfool/github_repos/julia/src/toplevel.c:884
jl_parse_eval_all at /home/learnedfool/github_repos/julia/src/toplevel.c:1010
jl_load_ at /home/learnedfool/github_repos/julia/src/toplevel.c:1057
include at ./boot.jl:367
include at ./compiler/compiler.jl:19
jl_apply at /home/learnedfool/github_repos/julia/src/julia.h:1760 [inlined]
do_call at /home/learnedfool/github_repos/julia/src/interpreter.c:117
eval_value at /home/learnedfool/github_repos/julia/src/interpreter.c:206
eval_stmt_value at /home/learnedfool/github_repos/julia/src/interpreter.c:157 [inlined]
eval_body at /home/learnedfool/github_repos/julia/src/interpreter.c:575
jl_interpret_toplevel_thunk at /home/learnedfool/github_repos/julia/src/interpreter.c:718
top-level scope at compiler/compiler.jl:68
jl_toplevel_eval_flex at /home/learnedfool/github_repos/julia/src/toplevel.c:884
jl_eval_module_expr at /home/learnedfool/github_repos/julia/src/toplevel.c:195 [inlined]
jl_toplevel_eval_flex at /home/learnedfool/github_repos/julia/src/toplevel.c:672
jl_toplevel_eval_in at /home/learnedfool/github_repos/julia/src/toplevel.c:936
eval at ./boot.jl:369
jl_apply at /home/learnedfool/github_repos/julia/src/julia.h:1760 [inlined]
do_call at /home/learnedfool/github_repos/julia/src/interpreter.c:117
eval_value at /home/learnedfool/github_repos/julia/src/interpreter.c:206
eval_stmt_value at /home/learnedfool/github_repos/julia/src/interpreter.c:157 [inlined]
eval_body at /home/learnedfool/github_repos/julia/src/interpreter.c:575
jl_interpret_toplevel_thunk at /home/learnedfool/github_repos/julia/src/interpreter.c:718
top-level scope at compiler/compiler.jl:3
jl_toplevel_eval_flex at /home/learnedfool/github_repos/julia/src/toplevel.c:884
jl_parse_eval_all at /home/learnedfool/github_repos/julia/src/toplevel.c:1010
jl_load_ at /home/learnedfool/github_repos/julia/src/toplevel.c:1057
jl_load at /home/learnedfool/github_repos/julia/src/toplevel.c:1070
exec_program at /home/learnedfool/github_repos/julia/src/jlapi.c:513
true_main at /home/learnedfool/github_repos/julia/src/jlapi.c:565
jl_repl_entrypoint at /home/learnedfool/github_repos/julia/src/jlapi.c:695
main at /home/learnedfool/github_repos/julia/cli/loader_exe.c:51
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
_start at /home/learnedfool/github_repos/julia/usr/bin/julia (unknown line)

make[1]: *** [sysimage.mk:61: /home/learnedfool/github_repos/julia/usr/lib/julia/corecompiler.ji] Error 1
make: *** [Makefile:82: julia-sysimg-ji] Error 2

@Pramodh-G Pramodh-G mentioned this pull request May 30, 2021
Closed
@simeonschaub
Copy link
Member

I added tests that were mutating

Maybe I am missing something, but I don't see any tests here where f has side effects.
Supporting CartesianIndex doesn't need to be part of this PR, let's keep that for a future PR.

@Pramodh-G
Copy link
Contributor Author

Pramodh-G commented May 30, 2021 •
edited
Loading

Oh, I maybe I misinterpreted what side-effects meant. Could you please provide some examples/cases?

I thought you just wanted confirmation that f gets called only when there is a BoundsError, And hence I added tests that actually changed values in the Matrix.

Since f cannot take in any arguments, it felt hard coming up with testcases where an existing global variable is mutated, since it might mess up other tests; If you want, maybe I could initialize some variables myself?

Supporting CartesianIndex doesn't need to be part of this PR, let's keep that for a future PR.

Sure, I'll remove those comments.

Thanks!

@simeonschaub
Copy link
Member

A .+ 1 does not modify A at all and you don't yet that it has been modified. The easiest way to test this is probably doing something like :

c = 0
@test get(() -> (c += 1; 0), ...) == ...
@test c == 1

@Pramodh-G
Copy link
Contributor Author

Sure, I'll try something like this out.

@vtjnash vtjnash merged commit 27aaadb into JuliaLang:master May 31, 2021
@mcabbott mcabbott mentioned this pull request May 31, 2021
Merged
@Pramodh-G Pramodh-G deleted the pramodh/get-tuple branch June 1, 2021 04:43
@mcabbott mcabbott mentioned this pull request Jun 2, 2021
Merged
shirodkara pushed a commit to shirodkara/julia that referenced this pull request Jun 9, 2021
Add get method, tests for Tuple and AbstractArrays (JuliaLang#41007)
21121af 
Replaces JuliaLang#40856
Fixes JuliaLang#40809
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
@johanmon
Add get method, tests for Tuple and AbstractArrays (JuliaLang#41007)
8700038 
Replaces JuliaLang#40856
Fixes JuliaLang#40809
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Pramodh-G @simeonschaub @vtjnash