[ty] Resolving Python path using `CONDA_PREFIX` variable to support Conda and Pixi #18267

lipefree · 2025-05-23T02:05:48Z

Summary

Implement what was discussed in astral-sh/ty#265. Pixi and Conda set the CONDA_PREFIX environment variable and not VIRTUAL_ENV. With this PR, if no --python flag has been passed and VIRTUAL_ENV has not been set then we check CONDA_PREFIX and resolve the path.

I am still not sure about how to implement: CONDA_PREFIX is checked only if there's no .venv directory in the project root.

My guess is it should be something like this but the .unwrap_or_else() is still confusing to me and I can't figure out how to check for CONDA_PREFIX after the .unwrap_or_else() .

SearchPathSettings {
            extra_paths: extra_paths
                .unwrap_or_default()
                .into_iter()
                .map(|path| path.absolute(project_root, system))
                .collect(),
            src_roots,
            custom_typeshed: typeshed.map(|path| path.absolute(project_root, system)),
            python_path: python
                .map(|python_path| {
                    PythonPath::from_cli_flag(python_path.absolute(project_root, system))
                })
                .or_else(|| {
                    std::env::var("VIRTUAL_ENV")
                        .ok()
                        .map(PythonPath::from_virtual_env_var)
                })
                .unwrap_or_else(|| PythonPath::Discover(project_root.to_path_buf())
                .or_else(|| {
                                    std::env::var("CONDA_PREFIX")
                                        .ok()
                                        .map(PythonPath::from_conda_prefix_var)
                                })),
        }

Closes astral-sh/ty#265

Test Plan

I reproduced the error from #265, and now there is no more lint:unresolved-import error. I am still not familar with the markdown tests but I will do more commits to provide tests.

This is my first opensource contribution and I am still a student with 0 experience in Rust, please give me constructive feedbacks! I will try to learn as quickly as I can!

github-actions · 2025-05-23T02:10:42Z

`mypy_primer` results

No ecosystem changes detected ✅

lipefree · 2025-05-23T13:22:18Z

crates/ty_project/src/metadata/options.rs

                        .map(PythonPath::from_virtual_env_var)
                })
-                .unwrap_or_else(|| PythonPath::Discover(project_root.to_path_buf())),
+                .or_else(|| Some(PythonPath::Discover(project_root.to_path_buf())))


If someone can help me out since I am new to Rust, this is the solution I came up with to deal with the compiler errors. Is it fine to wrap PythonPath::Discover(project_root.to_path_buf()) in Some(.) and is it fine to simply unwrap std::env::var("CONDA_PREFIX").ok().map(PythonPath::from_conda_prefix_var) ?

MichaReiser

Thanks for working on this. I left an inline comment. I hope the rest of the team agrees with my proposed change but that should simplify your life

MichaReiser · 2025-05-23T14:00:45Z

crates/ty_project/src/metadata/options.rs

                        .map(PythonPath::from_virtual_env_var)
                })
-                .unwrap_or_else(|| PythonPath::Discover(project_root.to_path_buf())),
+                .or_else(|| Some(PythonPath::Discover(project_root.to_path_buf())))


This may not work as intended (unless I'm misreading the code here).

The .unwrap_or_else will only look at the CONDA_PREFIX if the or_else on line 177 returned None. However, this can never be the case, because the or_else on line 177 always.

Given that CONDA_PREFIX is more explicit than an existing .venv directory, I think I'm leaning towards changing the order to

VIRTUAL_ENV if it is set

CONDA_PREFIX

Fallback to inspecting the project directory structure

That should also simplify your live because all you need to do is add an or_else similar to the VIRTUAL_ENV branch before the unwrap_or_else

Given that CONDA_PREFIX is more explicit than an existing .venv directory

What worries me here is that it's possible (though not necessarily recommended) to create a virtual environment inside a conda environment. If the user has done that, it might be pretty surprising if we require them to explicitly activate the virtual environment in order for us to recognise it (it goes against the general "Astral philosophy" of not requiring users to imperatively activate their virtual environments).

I say "activate a virtual environment" because I think almost no one sets the VIRTUAL_ENV environment variable manually: it's usually set as part of the virtual-environment activation (source .venv/bin/activate in the user's shell)

Fair enough. Implementing this with this specific discovery ordering would be much more complicated because it can't be implemented here. Instead, it would need to happen inside the search path setting resolution

(it goes against the general "Astral philosophy" of not requiring users to imperatively activate their virtual environments).

Fair, but it would be activate if we have a tigher uv integration

I doubt people who use conda environments are going to want to run ty via uv — pixi or conda are the project managers you probably want to use if you're using conda environments

Now I'm confused. if you use conda, then changing the order wouldn't matter? And I think it's fine to ask for explicit configuration if you have a project where you have an activated CONDA env and a nested venv. It's just the best information that the user has given us and a environment variable is, IMO, always more explicit than using some heuristic.

OK, yes, that's fair enough. If the user is using a non-conda virtual environment inside their conda environment, I guess in 2025 they might be using a non-conda project manager to run commands in their project and relying on that non-conda project manager to keep that virtual environment activated for the duration of those commands. I guess I'm wrapping my head round this paradigm too — I haven't used conda since before uv was a thing 😆

In that case, I'm OK with what you suggest: preferring an explicitly activated conda environment over a virtual environment unless the virtual environment has also been explicitly activated (either by the user or by a non-conda project manager such as uv/hatch/poetry/pdm).

We should make sure we write up the rationale for this in a comment, though!

I'm also very open to changing this if it proves problematic. The benefit of still being in preview :)

"The .unwrap_or_else will only look at the CONDA_PREFIX if the or_else on line 177 returned None. However, this can never be the case, because the or_else on line 177 always." so from what I understand, you are saying that even if no .venv is found, we will never reach line unwrap_or_else in line 178. From my testing, if no .venv directory is found then ty will set the python path using CONDA_PREFIX which means that unwrap_or_else in line 178 is used (It's my first time writing Rust so please correct me).

Also from what I understand from your conversations is that I should revert the changes of my last commit and have the following check order :

VIRTUAL_ENV if it is set

CONDA_PREFIX if it is set

Fallback to inspecting the project directory structure

Thank you for reviewing my PR!

lipefree · 2025-05-23T14:57:28Z

I was looking for a way to write tests by setting/unsetting the VIRTUAL_ENV and CONDA_PREFIX environment variables and having a .venv directory. But looking at (writing tests guide)[https://github.com/astral-sh/ruff/tree/main/crates/ty_test], I was not able to understand if the current TOML-based configuration format support my testing case.

MichaReiser · 2025-05-23T14:58:53Z

our markdown tests don't support setting environment variables. The best you can try is to write a CLI test in

ruff/crates/ty/tests/cli.rs

Line 11 in d098118

fn test_run_in_sub_directory() -> anyhow::Result<()> {

MichaReiser · 2025-05-23T15:24:46Z

I'll convert this to draft. @lipefree you can convert it back to "ready for review" once your PR is ready for another round. This makes it easier for me to know when I should take a look at your PR

lipefree · 2025-05-23T16:59:44Z

our markdown tests don't support setting environment variables. The best you can try is to write a CLI test in

ruff/crates/ty/tests/cli.rs

Line 11 in d098118

fn test_run_in_sub_directory() -> anyhow::Result<()> {

I tried my best for the CLI test. I don't know if it is enough for you.

MichaReiser

This is great and nice write up in the test!

MichaReiser · 2025-05-23T17:31:10Z

Oh no, the tests don't work on Windows because the venv path is slightly different. Do you want to take a look?

lipefree · 2025-05-23T17:37:47Z

Oh no, the tests don't work on Windows because the venv path is slightly different. Do you want to take a look?

I don't have a Windows machine so it is hard for me to look at how it works. I guess I just need to figure out how Conda environments are structured on Windows but then I don't know how to have OS dependent tests run on my mac (apart from letting the CI do it for me which is quite slow).

MichaReiser · 2025-05-23T17:40:59Z

I also don't have a windows machine ;(

Here's an example on how you can have custom logic based on whether you're on windows.

ruff/crates/ty/tests/cli.rs

Lines 957 to 961 in d098118

    
           let config_env_var = if cfg!(windows) { 
        
               "APPDATA" 
        
           } else { 
        
               "XDG_CONFIG_HOME" 
        
           };

AlexWaygood · 2025-05-23T17:42:23Z

crates/ty/tests/cli.rs

+            "#,
+        ),
+        (
+            "conda-env/lib/python3.13/site-packages/package1/__init__.py",


on Windows the path needs to be conda-env/Lib/site-packages/package1/__init__.py rather than conda-env/lib/python3.13/site-packages/package1/__init__.py

Does "conda-env/Lib/site-packages/package1/init.py" works instead of "conda-env/Lib/site-packages/package1/init.py" ? I am just trusting my copy/paste skill at this point since I can't test locally.

Ok it's just markdown doing some formatting. I got it.

I mistyped my message originally -- sorry! If you refresh your browser, the edited version of my message should be displayed :-)

It does indeed need to be __init__.py rather than init.py

Ok it's just markdown doing some formatting. I got it.

yeah, exactly 😄

I hope it will pass now and that I did not do any typo !

MichaReiser · 2025-05-23T17:59:08Z

Perfect! ty

AlexWaygood · 2025-05-23T18:05:28Z

@lipefree congrats on your first open-source contribution!! This is a fantastic feature for us to have 😃

lipefree · 2025-05-23T18:09:19Z

Thank you for your help @MichaReiser @AlexWaygood on my first contribution !

lipefree added 2 commits May 23, 2025 03:00

Minimal implementation to check variable to resolve python environment

aaeec28

Add type to enum

9e9d279

lipefree requested review from AlexWaygood, MichaReiser, carljm, dcreager and sharkdp as code owners May 23, 2025 02:05

AlexWaygood added the ty Multi-file analysis & type inference label May 23, 2025

lipefree changed the title ~~[Ty] Resolving Python path using CONDA_PREFIX variable to support Conda and Pixi~~ [ty] Resolving Python path using CONDA_PREFIX variable to support Conda and Pixi May 23, 2025

sharkdp removed their request for review May 23, 2025 07:28

Fix check order, if .venv is found then ignore

9ae3e26

lipefree commented May 23, 2025

View reviewed changes

MichaReiser approved these changes May 23, 2025

View reviewed changes

MichaReiser self-requested a review May 23, 2025 14:01

lipefree added 2 commits May 23, 2025 17:06

format to comply with CI/cargo fmt

e4c116e

Revert back to order : VIRTUAl_ENV, CONDA_PREFIX then .venv dir

97ecf3c

MichaReiser marked this pull request as draft May 23, 2025 15:24

Tests added in form of CLI tests

7932e17

lipefree marked this pull request as ready for review May 23, 2025 17:00

MichaReiser approved these changes May 23, 2025

View reviewed changes

AlexWaygood reviewed May 23, 2025

View reviewed changes

Fix test conda environment path to package for windows

ca64107

MichaReiser merged commit 53d19f8 into astral-sh:main May 23, 2025
35 checks passed

This was referenced Jun 2, 2025

Docs: list of externally defined variables respected by ty is out of date astral-sh/ty#564

Closed

Use .venv over base conda environment astral-sh/ty#611

Closed

JaRoSchm mentioned this pull request Jul 10, 2025

ty not detecting pixi environments if installed by pixi global astral-sh/ty#796

Closed

[ty] Resolving Python path using CONDA_PREFIX variable to support Conda and Pixi #18267

[ty] Resolving Python path using CONDA_PREFIX variable to support Conda and Pixi #18267

Uh oh!

Conversation

lipefree commented May 23, 2025 • edited by MichaReiser Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lipefree commented May 23, 2025

Uh oh!

MichaReiser commented May 23, 2025

Uh oh!

MichaReiser commented May 23, 2025

Uh oh!

lipefree commented May 23, 2025

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented May 23, 2025

Uh oh!

lipefree commented May 23, 2025

Uh oh!

MichaReiser commented May 23, 2025

Uh oh!

AlexWaygood May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented May 23, 2025

Uh oh!

Uh oh!

AlexWaygood commented May 23, 2025

Uh oh!

lipefree commented May 23, 2025

Uh oh!

Reviewers

Assignees

[ty] Resolving Python path using `CONDA_PREFIX` variable to support Conda and Pixi #18267

[ty] Resolving Python path using `CONDA_PREFIX` variable to support Conda and Pixi #18267

lipefree commented May 23, 2025 •

edited by MichaReiser

Loading

github-actions bot commented May 23, 2025 •

edited

Loading

`mypy_primer` results

MichaReiser May 23, 2025 •

edited

Loading

AlexWaygood May 23, 2025 •

edited

Loading

AlexWaygood May 23, 2025 •

edited

Loading