Accept optional argument in ArrayValue.__array__

[moble]

I ran into a situation when passing an SVector{4, Float64} to a python function where ArrayValue.__array__ was called with the optional dtype argument, which is standard for numpy, but caused an error here. Adding this option to the call made the conversion work.

[stevengj]

Ah looks like you ran into the same issue as I did earlier today. Presumably this PR fixes #160?

Maybe add a test using the test case in #160?

[moble]

That's a pretty great github jinx!

I've added the test. Testing this code path does require adding numpy as a dependency in CondaPkg.toml, but I think that's probably better anyway.

[stevengj]

Maybe instead do:

pymoduleexists(mod::AbstractString) = pyconvert(Bool, pyimport("importlib").util.find_spec(mod) != Py(nothing))

if !pymoduleexists("numpy")
   PythonCall.C.CondaPkg.add("numpy")
end

so that it only installs numpy if the test is run?

[moble]

I guess that makes sense,^(*) though it'll evidently alter the CondaPkg.toml used to run the tests, which means that people making PRs and such will need to be careful not to commit that change...

That pymoduleexists function looks so handy that, IMHO, it should be exported. I've added it to src/concrete/import.jl and exported it. But this is just my suggestion; I'm open to differing opinions.

I might even suggest that a call like that to CondaPkg would form a nice basis for extending pyimport to act more like the new Julia import — prompting a user to add a package if it's not found, maybe with options for version and channel info. The conversation by @icweaver and @sairus7 indicates there may be some interest in such functionality.

---

^(*) Except that it has to be pyimport("importlib.util").find_spec(m).

[stevengj]

I might even suggest that a call like that to CondaPkg would form a nice basis for extending pyimport to act more like the new Julia import — prompting a user to add a package if it's not found, maybe with options for version and channel info. The conversation by @icweaver and @sairus7 indicates there may be some interest in such functionality.

PyCall has something like that with pyimport_conda (though without a prompt), but I think that probably should be a topic for another issue?

[stevengj]

Suggested change

	@test !pymoduleexists(randstring(32))
	@test !pymoduleexists("rDJb5uy3eTorFw4c7UbcGOC59FjVGcnB") # randstring(32)

No reason to choose a different random string for each test run.

[stevengj]

(Then you can also remove the Random dependency.)

[cjdoris]

Thanks for the PR! Could you revert all the changes except those to array.jl please? I'm happy to discuss pymoduleexists in another ticket, though my gut is to just let the user call find_spec themselves. I don't mind this not being tested right now, this clearly works, and there is a lot of test writing to be done anyway.

[cjdoris]

I've just pushed the changes to __array__(). Happy to discuss the other changes on a separate ticket.