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

Fixing OSX makefile #17

Merged
merged 8 commits into from Mar 26, 2011
Merged

Fixing OSX makefile #17

merged 8 commits into from Mar 26, 2011

Conversation

fgb
Copy link
Contributor

@fgb fgb commented Feb 23, 2011

I updated a couple of links for fetch and added some stray files to clean. I'm also making a couple of suggestions regarding requiring PYVERSION in the command line and dropping the PREFIX from mpl_install. For this last one, I'm assuming that the dependencies are compile-time and matplotlib will work correctly from the relevant site-packages (I haven't seen any problems yet, although I've kept the dependencies in the PREFIX), but I welcome any comments or corrections.

Although a hard-coded PYVERSION=2.6 is safe for snow leopard, it might make more sense to leave it blank, defaulting to the 'python' command that takes precedence in the PATH. Also, suggesting in the documentation that it can be included in the command line might prompt users to choose the right version without having to edit the makefile.
@@ -9,5 +9,5 @@ lib, png or freetype on your system

Example usage::

PREFIX=/Users/jdhunter/dev make -f make.osx fetch deps mpl_install
PREFIX=/Users/jdhunter/dev [PYVERSION=2.6] make -f make.osx fetch deps mpl_install
Copy link
Member

Choose a reason for hiding this comment

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

The brackets that mean optional settings could be mistaken for syntax. I suggest reformulating this along these lines (note that you can set variables on the make command line):

make -f make.osx fetch deps mpl_install PREFIX=/Users/jdhunter/dev PYVERSION=2.6

Variables:
    PREFIX (required): where to install the dependencies
    PYVERSION (optional): <<<please describe the effect and the default>>>

The Python libraries will be installed in the usual location, not under PREFIX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip. I was just following standard man notation, but I understand your concern and added a couple commits to address this.

@jkseppan
Copy link
Member

jkseppan commented Mar 4, 2011

I would still like to get a comment from John Hunter on removing the --prefix argument.

@jkseppan
Copy link
Member

jkseppan commented Mar 6, 2011

Summary of changes in my commit:

  • export variables directly from make to avoid shell quoting issues; this includes a fix to ARCH_FLAGS
  • add check-prefix helper target to avoid accidentally compiling with -I/include/freetype2 etc
  • simplify mpl_build etc, add new targets for different ways of installing
  • add help as first target to avoid accidental make clean
  • document

I got "make binaries" almost working by disabling wxagg from the setup.cfg file and fudging some version numbers of the generated files. I don't think this review request broke anything there, it's just that my system doesn't have a good version of wxPython.

@jkseppan
Copy link
Member

jkseppan commented Mar 6, 2011

This won't merge cleanly to master. I'll prepare another pull request for that.

@jkseppan
Copy link
Member

In the absence of further comments, I'll merge this in.

@jkseppan jkseppan merged commit a71414d into matplotlib:v1.0.x Mar 26, 2011
@richbwood richbwood mentioned this pull request Dec 19, 2012
tacaswell pushed a commit that referenced this pull request Feb 18, 2016
efiring pushed a commit that referenced this pull request Sep 19, 2016
DOC: explain changes to boxplot styles
fariza added a commit to fariza/matplotlib that referenced this pull request Sep 22, 2016
fixing log10 in Dr_db and moving pyplot import into method
swfiua pushed a commit to swfiua/matplotlib that referenced this pull request Mar 17, 2017
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.

None yet

2 participants