Skip to content

updates for recent python and matplotlib#9

Open
John-Colvin wants to merge 5 commits intokoji-kojiro:masterfrom
John-Colvin:updates
Open

updates for recent python and matplotlib#9
John-Colvin wants to merge 5 commits intokoji-kojiro:masterfrom
John-Colvin:updates

Conversation

@John-Colvin
Copy link

Changes

  • Use an environment variable (MATPLOTLIB_D_PYTHON) instead of whatever python happens to be.
  • python3 doesn't support lowercase from string, so use ascii_lowercase instead (matplotlib functions are all ascii anyway)
  • recent matplotlib has a function called deprecated, which is a D keyword. By testing for invalid identifiers and appending an _ to them, we avoid this problem and any other similar ones that may come along.

@koji-kojiro
Copy link
Owner

Hi, @John-Colvin

What's the purpose of using $ MATPLOTLIB_D_PYTHON instead of system's python?
I think it's slightly complicated for users to set environment variables before installation.

@koji-kojiro koji-kojiro self-requested a review September 15, 2017 05:21
@John-Colvin John-Colvin force-pushed the updates branch 7 times, most recently from 8acba89 to 0044913 Compare September 19, 2017 11:58
@John-Colvin John-Colvin force-pushed the updates branch 2 times, most recently from 149650c to 567a1fb Compare September 19, 2017 12:22
"targetPath": "build",
"workingDirectory": "build",
"preBuildCommands": ["python $PACKAGE_DIR/python/prebuild.py $PACKAGE_DIR"]
"preBuildCommands": ["$MATPLOTLIB_D_PYTHON $PACKAGE_DIR/python/prebuild.py $PACKAGE_DIR"]
Copy link
Owner

Choose a reason for hiding this comment

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

I recognize that "preBuildCommands" is executed when users install this library by using dub. Is this wrong? If it's right, we had better not use any environment variables to specify the version of Python.

- python3-matplotlib

before_install:
- if [ $TRAVIS_OS_NAME = osx ]; then brew install python$PYTHON_VER; brew install homebrew/science/matplotlib; fi
Copy link
Owner

Choose a reason for hiding this comment

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

Travis seems to fail due to this line. It seems better to specify Python's version by using pyenv, but I have no experience on travis with pyenv. Can you solve this?

@TritiatedWater
Copy link

Maybe removing brew install python$PYTHON_VER will help?

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.

3 participants