Autover fails to read .version on Windows when param is stored on a "posix-like filesytem"

[ceball]

Code like vfile = os.path.join(os.path.dirname(self.fpath), '.version') can fail because you can end up with something like /path/to/param\\.version on windows. (I know, I know, windows...)

Code like the above appears in a few places, but reading the .version file is the one that bothers me:

https://github.com/pyviz-dev/autover/blob/956ffe8fef9dbf4dda67e2529be986e3df1cbda8/autover/version.py#L311-L324

I used quotes in the title because it doesn't have to be a filesystem - it could be something else...something like some kind of a database, say. As long as the "path separator" you need to use to open the .version file is different from what python thinks the path separator is for the platform it's running on (which could indeed be correct for accessing files on some other part of the system), it will not work out.

There are various things you could do to try to overcome this problem, but I think my proposed solution for this in the past was that .version should instead be _version.py, and autover should just rely on python's import system, which is guaranteed to be set up ok anywhere you're able to import stuff like param in the first place. But presumably there are downsides to that.

[jbednar]

I'd be happy to look for _version.py instead, particularly since it helps address the other situation of inappropriately calling git for released versions -- if we change the filename at this time, developers who happen to have .version sitting around on their hard drive from an old call to setup.py won't be messed up! (Assuming we change setup.py not to generate _version (or .version) unless actually building a release package)

[jlstevens]

I think the problem is that fpath is set from __file__ where ever the package declares its version and autover may be somewhere else with a different os.sep. Switching to _version.py seems to me like it will cause more complication and more potential for bugs that simply fixing path handling in one place. I believe that simply using:

        self.fpath = os.path.abspath(fpath)

On line 178 will make sure the separator is at consistent at which point I believe that Python will be able to handle opening the file just fine. Is there any reason you don't think this is true @ceball ?

[ceball]

I don't think that will work in the scenario I described (which I did not describe very well, sorry).

If param is at /path/to/param/__init__.py on windows (because e.g. param is installed to some place with "posix like semantics", i.e. not the typical windows "drive" you're familiar with), then I guess fpath would come back as "/path/to/param/__init__.py". I think os.path.abspath(fpath) would then become "X:\\path\\to\\param\\__init__.py" where X: is probably just the drive from os.getcwd() or something like that. But the path to get any param file e.g. .version really is /path/to/param/.version.

But you still might be able to figure out a way to get the path separators right!

However, I only gave the particular example in this issue because it was a recent concrete one in my mind. There are probably various others I could think up, since as far as I know, a python module does not have to be a file "according to the language rules" (not sure though! and often they are referred to as files). E.g. __file__ does not have to be set to anything, according to https://docs.python.org/3/reference/datamodel.html:

(not sure it lists all the istuations when it doesn't have to be set?)

Anyway, maybe a less weird example than the one I posted in this issue would be when a module is imported from a zip file? That's functionality in the stdlib (right?) and I think you might have trouble using open() based on __file__. Not sure. We could probably write a test for this case to show it, since the test wouldn't depend on any special setup.

Basically my conclusion in the past was: what can you trust in the place you're running except the import system, given that nothing would work without that (right?!). But there are things in/around setuptools/other(newer) stdlib modules to help with "reading 'data' from packages". An oldish setuptools example would be using something like pkg_resources.resource_string(__name__, ".version") - that or a better equivalent that doesn't need setuptools might work. I don't remember this too well (and guess where I remember it from...packaging examples...another aaargh!! memory ;) ). Maybe the situation is a lot better now than when I last investigated, particularly with the dropping of py2?

[ceball]

https://docs.python.org/3.8/library/importlib.html

[jlstevens]

If importlib.resources was available before version 3.7 then I would say that it is the right way to go.

Using the import system to simply grab some data from a file seems rather ridiculous although it does seem to resolve some of these annoying edge cases as you point out. I suppose instead of a JSON file, it can simply be a Python dictionary literal that you import but again, this all seems rather silly.

[jbednar]

importlib.resources does look suitable, but it's about 10 years too late! Basically, what we're up against is that the default of simply setting an attribute __version__ = "1.2.3" will always simply work because it's just a line of code in a Python file. Something like versioneer will also always work because when it's done rewriting the code, you're left with just a line of code in a Python file. So here, I think the only way we can be similarly reliable and trustworthy is simply to be a line of code in a Python file, which in this case autover would generate for the package to be distributed with. It's not self-modifying code as for versioneer, because the entire file (not search and replace) would be generated, but it's very little different from what happens now except that it's an importable file and not a file we could fail to read. To me it seems like _version.py is going to reliable and trustworthy, in any cases that the underlying module is itself loadable, while requiring only a tiny tweak to how autover works.