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

switching to PythonCall.jl #32

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

icweaver
Copy link
Member

@icweaver icweaver commented Jan 15, 2022

Trying out PythonCall.jl for our Python-based tests here. It uses scratch spaces and self contained envs, which is chef's kiss, and I think activating already existing envs is on the creator's to-do. If we can get this to play nice with CI in terms of caching, should we just make the switch entirely over from PyCall.jl?

At first, needing to do things like explicitly indicating what Python packages we want in global scope and applying conversions to Py objects manually seemed like a trade-off to me, but the more I use it, the more it is starting to grow on me as a self-documenting feature

To-Do

@icweaver icweaver changed the title switching to PythonCall.jl switching to PythonCall.jl Jan 15, 2022
@codecov
Copy link

codecov bot commented Jan 15, 2022

Codecov Report

Merging #32 (c0d0e64) into main (16fc5c9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #32   +/-   ##
=======================================
  Coverage   91.70%   91.70%           
=======================================
  Files          14       14           
  Lines        1181     1181           
=======================================
  Hits         1083     1083           
  Misses         98       98           

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 16fc5c9...c0d0e64. Read the comment docs.

@mileslucas
Copy link
Member

This looks like it's worth a look! Thanks for digging into it, Ian!

Decide on CondaPkg.toml location?

I think you can just put it in test/CondaPkg.toml and it will sync with the PythonCall.jl used in the test environment. This is how I did it with the benchmarks here, and it created a virtualenv in the bench folder (which I added to .gitignore)

@icweaver
Copy link
Member Author

icweaver commented Jan 15, 2022

Sounds good!

I think you can just put it in test/CondaPkg.toml and it will sync with the PythonCall.jl used in the test environment

Oh, I was hoping that would work! I think I am doing something incorrectly though because PythonCall.jl doesn't seem to find it there and just falls back to the CI temp env I think:

CondaPkg Creating environment
     /home/runner/.julia/scratchspaces/0b3b1443-0f03-428d-bdfb-f27f9c1191ea/install/micromamba
     -r
     /home/runner/.julia/scratchspaces/0b3b1443-0f03-428d-bdfb-f27f9c1191ea/root
     create
     -y
     -p
     /tmp/jl_vuX7W8/.CondaPkg/env
     --no-channel-priority
     python >=3.5,<4
     -c
     conda-forge

@mileslucas
Copy link
Member

oh, I see. I haven't used it on CI. In that run, though, is the CondaPkg.toml in the root folder or the test folder?

@icweaver
Copy link
Member Author

Test folder. Oh, I've been trying it locally too and seem to be getting the same issue. Only having it in the root folder seems to work for now, but I can double check

@icweaver
Copy link
Member Author

Also started a discussion here, but could just be jumping the gun

JuliaPy/PythonCall.jl#96

@mileslucas
Copy link
Member

Good snooping, we'll need some sorcery to overcome the "magic" 🪄

@@ -1,3 +1,4 @@
[deps]
python = "3.9"
Copy link
Member Author

Choose a reason for hiding this comment

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

Attempting to install numpy on windows with python 3.10 seems to be running into this, so staying on 3.9 for now

@icweaver
Copy link
Member Author

icweaver commented Jan 16, 2022

So I fell down a rabbit hole of package tooling lore. I guess the dream of having truly separate project and test envs just isn't there quite yet. Until #1233 comes, how do you feel about going "old school" with the setup in c0d0e64?

Having a top-level view of everything in a single Project.toml feels kind of nice, but that might just be the Stockholm syndrome talking at this point

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.

2 participants