search

mandeep / sublime-text-conda

Work with conda environments in Sublime Text 3
BSD 3-Clause "New" or "Revised" License
26 stars 10 forks source link

Fixes windows PATH issue causing import errors with some packages. #15

Closed jim-hart closed 4 years ago

jim-hart commented 4 years ago

Addresses #12

If /env/Library/bin is not on the Windows PATH, packages are unable to load dependencies in that directory. Issue is addressed by extending ExecuteCondaEnvironmentCommand with context manager protocols that temporarily prepend the required directory to os.environ['PATH'] prior to running the target file. The PATH entry is reset to its original value once target file exits.

The patch is only applied to Windows environments where conda version > 4.6

I don't know if using regex to parse conda's version is the best way to handle retrieving the desired information, but I felt it was better then looking for the meta-data file. Please let me know if an alternative is preferred.

mandeep commented 4 years ago

Thanks for the PR! I don't mind the regex, but a more robust method may be to call conda info --json and then parse the version key. Everything else looks great. I'll test this out on my own install later today.

jim-hart commented 4 years ago

No problem! Thanks for the suggestion on using conda info --json, that is more robust. I pushed a new commit to this branch to include the change.

mandeep commented 4 years ago

Thanks for taking the time to make the change! One thing I noticed though is that line 489 is performing the check self.conda_version > (4, 6) when I think it should be self.conda_version >= (4, 6)

jim-hart commented 4 years ago

Ahh, my mistake. I pushed a commit that fixes the comparison.

mandeep commented 4 years ago

Perfect! Thanks so much for this!

jim-hart commented 4 years ago

Its no problem; I use the package daily so I'm happy to help where I can.

Local use has been working great so far, however, I just caught a small side-effect resulting from the version-number check. There is a noticeable hang every time I execute the build process; I narrowed it down to the subprocess call that gets conda's version number via conda info --json.

This is more of an annoyance than anything else (calling the activation script caused a noticeable hang as well). A simple fix would be lazily load the version number and cache it in a class attribute.

I'm can submit a PR to address it, but I don't know if its worth the review process as it only affects Windows users and I'm the only one reporting it right now. Regardless, I wanted to at least make note of it here in case someone reports it in the future.

Anyway, thanks for all your help!

mandeep commented 4 years ago

This sounds like something that's worth a PR if you're willing to submit one! 😃

It's true that subprocess can cause noticeable hangs so any optimization is more than welcome. Even though I'm not a Windows user myself, I believe most users of this plugin are using Windows so I'm sure they would much appreciate this!

jim-hart commented 4 years ago

Always happy to submit a PR (especially when I'm the baddie 😛).

I pushed PR #16 to address issue.