API: Environment: create_environment is not deterministic

[blueyed]

There is no recommended API to explicitly create an Environment from an executable.
create_environment currently falls back to based on if something is a file:

def create_environment(path, safe=True):
    """
    Make it possible to manually create an environment by specifying a
    Virtualenv path or an executable path.

    :raises: :exc:`.InvalidPythonEnvironment`
    :returns: :class:`Environment`
    """
    if os.path.isfile(path):
        _assert_safe(path, safe)
        return Environment(_get_python_prefix(path), path)
    return Environment(path, _get_executable_path(path, safe=safe))

This means that the caller needs to do the same check before to ensure that path is treated at the executable.

I suggest that either only executables are supported, or kwargs (executable, path) get used, or distinct functions should be created.

As for jedi-vim I can use Environment() directly - but its docstring explicitly states that it should not be used directly:

class Environment(_BaseEnvironment):
    """
    This class is supposed to be created by internal Jedi architecture. You
    should not create it directly. Please use create_environment or the other
    functions instead. It is then returned by that function.
    """

(I am refactoring the environments currently - on top of #1108)
Related comment there: #1108 (comment)

[davidhalter]

Yeah. I agree. I actually thought about the fact that this was not the best idea (to have an argument that does multiple things) and now we're stuck in that mess. I should have reacted then and there, but now it's kind of too late. I guess we should just add a new function create_environment_from_file or move current behavior away from it.

Not sure what the best move is in terms of naming, but I'm sure that we want to move away from the status quo.

[micbou]

Sorry, that's my fault. I think the function should be deprecated in favor of two new functions create_environment_from_file (or create_environment_from_exe?) and create_environment_from_venv. This could be done for release 0.12.1 then create_environment would be removed in 0.13.0.

[davidhalter]

@micbou No problem. I didn't really spot it either.

[blueyed]

Cannot we use new kwargs for it?

executable => use file
venv => use dir
path => do both (deprecated maybe, but there might be use cases)

[davidhalter]

I just realized again that it's not as bad as I thought it was originally. I thought it would also resolve stuff like python3.5 to a path (which would make it not deterministic).

So it's actually better than I thought. I think this is deterministic, because it will behave just fine for the user. It might even benefit users, because they usually don't care about the bin/python in virtualenvs and if they do that's also fine. I fail to see that this is a big enough issue to deprecate something (or add new functionality).

I agree that it's maybe not ideal and ambiguities are generally not that cool, but it's also really practical for a lot of users.

[blueyed]

Yes, for the end user it is good, but I came across it when using it in jedi-vim (where I then just used Environment directly, which is fine in that case).
Feel free to close this for now.

[davidhalter]

Thanks for clarifying. I really think that it's fine for now. Let me know if it's a bigger issue or you think that it's something we should rethink.