Simplify setup.py's get_setup_version()?

[ceball]

Unless I'm confused, I don't see how the "autover unavailable" warning at https://github.com/ioam/autover/blob/master/setup.py#L45 will ever be reached. If things fall through to embed_version(), then embed_version() will either work and so version will not be None, or embed_version() will fail and there'll be an exception.

def embed_version(basepath, ref='v0.2.1'):
    """
    Autover is purely a build time dependency in all cases (conda and
    pip) except for when you use pip's remote git support [git+url] as
    1) you need a dynamically changing version and 2) the environment
    starts off clean with zero dependencies installed.
    This function acts as a fallback to make Version available until
    PEP518 is commonly supported by pip to express build dependencies.
    """
    import io, zipfile
    try:    from urllib.request import urlopen
    except: from urllib import urlopen
    response = urlopen('https://github.com/ioam/autover/archive/{ref}.zip'.format(ref=ref))
    zf = zipfile.ZipFile(io.BytesIO(response.read()))
    ref = ref[1:] if ref.startswith('v') else ref
    embed_version = zf.read('autover-{ref}/autover/version.py'.format(ref=ref))
    with open(os.path.join(basepath, 'version.py'), 'wb') as f:
        f.write(embed_version)

def get_setup_version(reponame):
    """
    Helper to get the current version from either git describe or the
    .version file (if available).
    """
    basepath = os.path.split(os.path.abspath(__file__))[0]
    version_file_path = os.path.join(basepath, reponame, '.version')
    version = None
    try: version = importlib.import_module(reponame + ".version") # Bundled
    except:  # autover available as package
        try: from autover import version
        except:
            try: from param import version # Try to get it from param
            except:
                embed_version(basepath)
                version = importlib.import_module(".version")

    if version is not None:
        return version.Version.setup_version(basepath, reponame, archive_commit="$Format:%h$")
    else:
        print("WARNING: autover unavailable. If you are installing a package, this warning can safely be ignored. If you are creating a package or otherwise operating in a git repository, you should refer to autover's documentation to bundle autover or add it as a dependency.")
        return json.load(open(version_file_path, 'r'))['version_string']

Was the intention to wrap embed_version() in try/except? Or do we not need the warning, because that situation should never happen (i.e. if embed_version() fails, there should be an error)?

[jlstevens]

Was the intention to wrap embed_version() in try/except?

That was the intention.

Or do we not need the warning, because that situation should never happen (i.e. if embed_version() fails, there should be an error)?

I don't mind either way. The difference is between getting a warning when embed fails or getting a traceback - I'm not sure what is preferable.

[jbednar]

I'm confused; why would autover code ever mention 'param'?

[jlstevens]

This is example code for other projects to adapt and use.

[jbednar]

Even so, param has nothing to do with autover, so I don't think we should be mentioning it in our example code. It's fine for us to use this code in param-based projects, and param could include that sample code, but I really don't think it belongs in autover.

[ceball]

why would autover code ever mention 'param'?
This is example code for other projects to adapt and use.

Giving a little more detail (Jean-Luc please correct me if I'm wrong):

Every project using autover needs to have a get_setup_version() function in its setup.py. There are various scenarios for using autover; the get_setup_version() function in autover currently covers all of them so the function can be copy/pasted as is into any other project, no matter how that project is using autover. The alternative, as I think you're suggesting, is to provide somewhere (docs/examples) multiple different get_setup_version() functions for people to choose from depending how they use autover. Originally, during development, it was much easier to have one get_setup_version(), because the function was being modified/figured out, so copy/paste everywhere was easier. Now that development work is finished, it seems like it would be a good idea to provide different versions for the different scenarios.

---

However, whether having all those ways of using autover is a good idea or not is also a question I think we should ask at some point.

I'd like to propose (one more time - the last time, I promise) that we reduce the number of ways of using autover - and that specifically we do not include autover in the param package. (The original version.py could stay in param until 2.0 or something for backwards compatibility).

I think it would simplify things to say the expected way for your-package to use autover is:

• copy autover.py into your project's git root directory and commit to git.
• import autover and call autover.get_setup_version(...) in your setup.py to get up to date version
• make sure at package time that the .version json file is included in your-package by adding your-package/.version to MANIFEST.in (or doing whatever you prefer in setup.py to achieve that). (version.py will be included automatically unless you do something to stop that.)
• put the following in your-package/__init__.py

try:
    import autover
    __version__ = autover.Version(...)
except:
    import os, json
    __version__ = json.load(... '.version')

The first path above ("import autover") is for developers who are either in the git repository, or have done a python setup.py develop install, or have the git repository on their PYTHONPATH. I.e. if import your-package from the git repo is possible, so is import autover.

The second path is for people who are installing a your-package package: no autover present or needed, because a .version file is present in the package.

I think the above scheme would work, unless I overlooked something.

I admit the downside of this scheme is that there'll be multiple copies of version.py around. From a code duplication point of view that doesn't seem loads worse to me than having multiple versions and copies of the "setup.py support functions" around. I think there's an improvement in clarity/ease of writing/maintaining project setup.py files (they won't be full of 'mysterious' slightly varying functions that inevitably get copy/pasted/modified a bit accidentally over time between projects). Instead, it would just be a case of keeping the single file autover.py up to date, which would contain everything necessary. (And we would expect to change that file very rarely anyway.)

However, it would also mean that in general when you do "import autover", autover would come from whichever package is first on your path.

Anyway, I assume that (as in the past) nobody else likes this suggestion of preferring multiple copies of autover.py to importing autover via the param package and having multiple copies of setup.py functions, so I'll proceed as if that's the case unless anyone agrees with me...

[jlstevens]

Every project using autover needs to have a get_setup_version() function in its setup.py. There are various scenarios for using autover; the get_setup_version() function in autover currently covers all of them so the function can be copy/pasted as is into any other project, no matter how that project is using autover.

That is correct.

As for the rest of your suggestion, I don't have the energy right now to consider a change of plans. One thing I do know that copying and pasting code is something you should always avoid if you can - and we know how to avoid it here.

[jbednar]

Personally, I think that what Chris outlines there should be what autover itself recommends, which is in line with my own comment that no autover code should be suggesting anything to do with param.

But I don't think that's relevant to what we should do in our own param-based projects. Param will itself need something to handle its versions, and so I think param should follow precisely what is suggested there, i.e. that it should include autover.py. But then I think we should stick with our plan of also shipping autover.py in Param and using it from there in our param-based projects, because that will give us a single shared copy to maintain for all our projects, simplifying our own lives. In our projects, I don't think get_setup_version should ever be trying to get it from an installed package; we should simply know that it's in param and get it from there. That way normal non-param autover users don't have to worry about param, param-based packages don't have to worry about autover, and everybody is happy.

[jlstevens]

Just to say I am in agreement with @jbednar on this. I don't mind if param isn't mentioned/recommended anywhere in autover itself.

[ceball]

I don't think get_setup_version should ever be trying to get it from an installed package; we should simply know that it's in param and get it from there.

Just to check, you mean: we should simply know that it's in param and get it from there if param is already installed; if param is not installed, we should download a copy of autover from github and use that.

E.g. say your-package depends on param. When you start in a clean environment and try to e.g. python setup.py develop inside your-package, setup.py can't get anything from param because there is no param.

Or am I confused?

I'm proceeding as if I'm not confused, i.e. embed_autover() remains a necessary fallback for projects that do not bundle autover until there's widespread availability of a pip that supports expressing "setup.py dependencies".

Let me know if I am confused...

[jbednar]

Urgh. No, you aren't confused; I just really don't like having to think about what happens during setup.py. And I really don't like the idea that setup.py would have to download autover from github; seems like that will give us very obscure and hard to track down headaches someday.

So is there not a version of pip that we ourselves as developers can use that would support expressing setup.py dependencies? I'd be happy with that, i.e. that people who want to develop on our projects have to use a particular version of pip.

Or else can our packages simply abort in setup.py saying that param needs to be installed before this will succeed? Even that seems preferable to fetching and installing a separate version of autover from within setup.py.

[jlstevens]

Just to check, you mean: we should simply know that it's in param and get it from there if param is already installed; if param is not installed, we should download a copy of autover from github and use that.

That's right. You still need the bit for embed for when you are in a clean environment. In all other cases, param is a required dependency and should therefore be available.

[jlstevens]

@jbednar I think the docstring of embed_version answers your question. In short, we need to wait for PEP518 to be be commonly supported by pip.

[jlstevens]

For reference, here is PEP518. Unfortunately, we need to use embed_version until that mechanism is widely supported.

[jbednar]

No, that doesn't answer my question; I'm asking why we care whether it is "widely supported" or "commonly supported". Can't we just settle for "supported"? Why do we need to worry about what other developers have on their machines, when writing code that we ourselves are the maintainers of? Or are such dependencies not in fact supported by any pip version?

[jlstevens]

Unfortunately, this is needed by users using pip + git until PEP518 is widely supported for users of pip (not just developers, otherwise I totally agree with you).

[jbednar]

Drat.

[jlstevens]

I've filed #32 as a reminder to try out the PEP518 approach with recent pip versions. This should offer a way to get rid of embed_version.

[ceball]

embed_version() caused much confusion while trying to test autover in the past, so I'll be glad to see it go when there's a practical alternative.