Skip to content

MNT : add guard to not look up HOME env on windows #3829

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 21, 2014
Merged

MNT : add guard to not look up HOME env on windows #3829

merged 1 commit into from
Nov 21, 2014

Conversation

tacaswell
Copy link
Member

This variable should not be defined on windows so don't
even bother looking at it.

addresses #3804

Suggested by @jbmohler

@WeatherGod
Copy link
Member

I am curious... I would imagine that it would be set if you use a cygwin
environment. What platform gets returned in python when executed in cygwin?

On Fri, Nov 21, 2014 at 3:43 PM, Thomas A Caswell notifications@github.com
wrote:

This variable should not be defined on windows so don't
even bother looking at it.

addresses #3804 #3804

Suggested by @jbmohler https://github.com/jbmohler

You can merge this Pull Request by running

git pull https://github.com/tacaswell/matplotlib HOME_win32_guard

Or view, comment on, or merge it at:

#3829
Commit Summary

  • MNT : add guard to not look up HOME env on windows

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#3829.

@tacaswell
Copy link
Member Author

It reports it's self as 'cygwin'

https://docs.python.org/2/library/sys.html#sys.platform

@WeatherGod
Copy link
Member

Ah, I didn't think the docs would list cygwin. Ok, then this looks good to me. Waiting for Travis to finish.

matthew-brett
matthew-brett reviewed Nov 21, 2014
View reviewed changes
lib/matplotlib/font_manager.py
@@ -142,7 +142,7 @@
""
]

if not USE_FONTCONFIG:
if not USE_FONTCONFIG and sys.paltform != 'win32':
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in 'platform'

Copy link
Contributor

Choose a reason for hiding this comment

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

How about using os.path.expanduser('~')?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed and forced....

@tacaswell
MNT : add guard to not look up HOME env on windows
09e1351 
This variable should not be defined on windows so don't
even bother looking at it.

addresses matplotlib#3804
@WeatherGod
Copy link
Member

And that is why we have automated tests to catch reviewer mistakes like those. I am presuming that it would be useless to do an expand_user('~') here because the code is building up search paths for X11 and Mac font locations. IIRC, Windows has a completely different font search mechanism.

WeatherGod added a commit that referenced this pull request Nov 21, 2014
@WeatherGod
Merge pull request #3829 from tacaswell/HOME_win32_guard
fed39c8 
MNT : add guard to not look up HOME env on windows
@WeatherGod WeatherGod merged commit fed39c8 into matplotlib:v1.4.x Nov 21, 2014
@tacaswell tacaswell deleted the HOME_win32_guard branch November 22, 2014 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tacaswell @WeatherGod @matthew-brett