Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add conda build recipe and improve version numbering#52

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
cwrowley merged 6 commits intopython-control:masterfromcwrowley:conda-build
Apr 6, 2015

Conversation

cwrowley
Copy link
Contributor

This pull request adds a "recipe" for building a conda package, and also improves how versions are specified for releases. Previously, one needed to specify the version number of each release manually in setup.py, and this has led to some confusion, as the version number did not necessarily correspond to tags in the git repository. With this pull request, version numbers are specified simply by pushing a tag to the git repo, as

git tag -a 0.6.6

The version numbers are then generated by a scriptmake_version.py, which is run automatically by the conda recipe (but not bysetup.py, so one still needs to remember to runmake_version.py before uploading to PyPI). The version numbers are consistent withPEP 440, so for instance, a version that is 6 commits after the version tagged as 0.6.6 would be version "0.6.6.post6".

The benefits of this approach are that the version numbers are specified only once (as a tag in the git repo), and they are guaranteed to correspond to the git repository. This avoids some confusion, as happened with v0.6.6 that is currently on PyPI (which I believe was built from@jgoppert's fork and includes PR#38, which has not been merged in, and is probably superfluous now).

This PR also changes the Travis CI build to test the conda recipe, and additionally to install slycot from a binary package I uploaded to binstar.org. This dramatically speeds up the travis build, which was spending most of its time compiling SLICOT from source.

There was a lot of unnecessarily complex logic in setup.py, taken fromnumpy, about whether to use setuptools or distutils.  In addition, theprocedure for generating the version information was only partiallyautomatic (one still needed to change the version manually in setup.py),and was somewhat unreliable (see issuepython-control#37).This commit simplifies setup.py, always using setuptools, as recommendedin the Python Packaging Guide:http://python-packaging-user-guide.readthedocs.org/en/latest/current.htmlThe version information is now generated directly from tags in the gitrepository.  Now, *before* running setup.py, one runs    python make_version.pyand this generates a file with the version information.  This is copiedfrom binstar (https://github.com/Binstar/binstar) and seems to work well.
To build the conda package:  conda build conda-recipeNote that the version is automatically set according to the filesgenerated by make_version.py, which is automatically run by the recipe
The majority of time in the Travis CI testing was spent installingslycot from source.  (This also required installing a fortran compileras part of the Travis build.)  Installing from a binary saves a lot oftime.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.86% when pulling5ff196c on cwrowley:conda-build into86ebb1c on python-control:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.86% when pulling5ff196c on cwrowley:conda-build into86ebb1c on python-control:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.86% when pulling5ff196c on cwrowley:conda-build into86ebb1c on python-control:master.

@slivingston
Copy link
Member

Ifcontrol/_version.py is missing (or otherwise if anIOError exception is raised while trying to read from it), then the version string is "dev", which does not conformwith PEP 440.

@slivingston
Copy link
Member

My comment above applies also if__version__ fails to be found when executingcontrol/_version.py. Cf.line 7 of setup.py.

@cwrowley
Copy link
ContributorAuthor

That's correct. And in that case, setup.py gives a UserWarning that 'dev' is an invalid version. This seems like a reasonable fallback to me, but do you have another suggestion?

@slivingston
Copy link
Member

Another concern: counting commits since a tag will fail in the case of branching. E.g., a version string of "0.6.6.post6" could occur for two distinct commits that have a common ancestor. While you make the commit hash available viacontrol.__commit__, why not include it in the version string?

@slivingston
Copy link
Member

My basic argument against not including some--possibly incomplete--version number in the source tree is that Git becomes a required dependency ofpython-control. E.g., a snapshot of the repo created viagit archive will not have any version specifier (except possibly the name of the directory, depending on howgit archive is called), and it will not be possible to install from that snapshot. This especially matters because GitHub provides a Web service likegit archive, e.g.,https://github.com/python-control/python-control/releases

@murrayrm
Copy link
Member

While we sort out the version discussion, is it worth separating out the two aspects of this request and re-issuing a separate pull request for all but the versioning part? Just a thought.

@cwrowley
Copy link
ContributorAuthor

@slivingston, the main reason I did things the way I did them is that I just copied what was done in binstar! I agree about the ambiguity of branches, but there shouldn't actually be any branching in the main releases, so that does not bother me.
About the dependency on git, I don't think it's quite accurate to say that Git becomes a required dependency of python-control. You need git if you want torelease a version (e.g., upload to PyPI or conda). But if you are justinstalling with pip or conda, you don't need git, because the version is baked into these distributions. So the dependency only affects those who are doing the releasing, or are doing something very nonstandard like installing from a git archive. Also, note that the current setup.py (which was copied from numpy) does try to callgit rev-parse HEAD, so if anything this change makes us less dependent on Git.
So I agree, it's not perfect, but I think the convenience and fool-proofness of just needing to specify the version once outweigh these drawbacks, which seem to only affect obscure situations. We do have an actual situation that has come up (namely, that the version currently on pypi does not match any version in the repository), and this solution would fix that and make sure things like that do not happen again.

@murrayrm, I suppose it would be possible to separate this into two pull requests, but I would have to change the conda configuration, and I'd rather not go to extra work to replace that with something that, in my opinion, does not work as well.

@slivingston
Copy link
Member

As I recall, the original mistake concerning the version on PyPI occurred because someoneuploaded a release without discussing it, froma fork of the main repo. Indeed, examine the version number insetup.py of that fork and compare withthe one on PyPI. Such could happen again, whether or not the version number is explicitly in the source tree.

Whilesetup.py from NumPy will try to callgit, it will append to the version number ".dev0+Unknown" if that fails. I am not sure that it is appropriately described as being more or less dependent on Git than the changset of this PR.

Nonetheless, I think my concerns about dependence on Git and duplicate version strings for distinct (non-release) commits can be addressed empirically. That is, perhaps we should just merge this PR, and try it for awhile.

By the way, besidesconda andpip, another major venue for getting python-control releases ishttps://sourceforge.net/projects/python-control/files/.

@cwrowley
Copy link
ContributorAuthor

While setup.py from NumPy will try to call git, it will append to the version number ".dev0+Unknown" if that fails. I am not sure that it is appropriately described as being more or less dependent on Git than the changset of this PR.

All I meant was that in the new version,setup.py does not call git at all. The only time we call git is when distributing (runningmake_version.py).

Note that the numpy stuff has been unreliable for us (e.g.,#37), and the fix using the__NUMPY_SETUP__ global variable is even called "ugly" and "hackish" in the numpy source. The binstar approach just seems simpler and cleaner to me, since it clearly separates the "pre-build" (make_version.py) from the actual build (setup.py), instead of trying to do both of these things insetup.py.

That is, perhaps we should just merge this PR, and try it for awhile.

Great. So unless anybody has any objections, I'll plan to do the following on Monday:

  • Merge this PR (unless somebody else wants to)
  • Tag the new version as 0.6.7
  • Upload it to PyPI and conda/binstar

I'm not one of the developers on sourceforge, but I can send Richard or Scott the tarball or you could grab it from PyPI and upload to sourceforge.

@murrayrm
Copy link
Member

All sounds good to me. When we do a new release on SourceForge, we typically send out an e-mail to python-control-announce describing what is new in the release. Scott and I can handle that as well, with input from others if we need help in the descriptions. The e-mail for the last release is here:

http://sourceforge.net/p/python-control/mailman/message/32135649/

@cwrowley
Copy link
ContributorAuthor

Looking back, I see version 0.6a was released in Nov 2012, more than 2 years ago. There have been a lot of changes since then (including changing the version numbering scheme from 0.6d, etc, to 0.6.5, etc), so I'm thinking it makes sense to bump the version to 0.7.0. I'll plan on releasing this as 0.7.0 tomorrow, unless there are any objections.

@murrayrm
Copy link
Member

Sounds sensible.

cwrowley added a commit that referenced this pull requestApr 6, 2015
Add conda build recipe and improve version numbering
@cwrowleycwrowley merged commit15fd5c1 intopython-control:masterApr 6, 2015
@cwrowleycwrowley deleted the conda-build branchApril 6, 2015 11:04
Sign up for freeto 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.

4 participants
@cwrowley@coveralls@slivingston@murrayrm

[8]ページ先頭

©2009-2025 Movatter.jp