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

handle $PATH not being set in python library #3845

Merged
merged 1 commit into from
Nov 6, 2018

Conversation

JelleZijlstra
Copy link
Contributor

Fixes #3844

@codecov-io
Copy link

codecov-io commented Oct 31, 2018

Codecov Report

Merging #3845 into master will increase coverage by 0.19%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3845      +/-   ##
============================================
+ Coverage     51.85%   52.05%   +0.19%     
  Complexity      203      203              
============================================
  Files           181      181              
  Lines         14360    14364       +4     
  Branches        495      495              
============================================
+ Hits           7447     7477      +30     
+ Misses         6675     6649      -26     
  Partials        238      238
Impacted Files Coverage Δ Complexity Δ
python-package/xgboost/core.py 82.02% <66.66%> (+4.23%) 0 <0> (ø) ⬇️
python-package/xgboost/compat.py 90.9% <0%> (+1.81%) 0% <0%> (ø) ⬇️

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 87f4999...d525216. Read the comment docs.

@RAMitchell
Copy link
Member

In what situations is path not set?

@JelleZijlstra
Copy link
Contributor Author

Our code was running a subprocess with a restricted environment (passing only those environment variables that were necessary).

@RAMitchell
Copy link
Member

PR looks fine, we can merge it after 0.81 release

Copy link
Collaborator

@hcho3 hcho3 left a comment

Choose a reason for hiding this comment

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

LGTM

@hcho3 hcho3 merged commit d9642cf into dmlc:master Nov 6, 2018
@hcho3
Copy link
Collaborator

hcho3 commented Nov 6, 2018

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Feb 5, 2019
@JelleZijlstra JelleZijlstra deleted the patch-1 branch September 15, 2023 23:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants