reorganize and update conda/python install docs #79
Conversation
There was a problem hiding this comment.
Looks reasonable to me. Just two minor issues to fix that I saw.
Are we abandoning any support for straight pip requirements support, or just relegating it to the advanced users that want to peer under the hood (e.g. inside the Dockerfile or some such).
Should we make an issue to track fixing up our build pipelines to do the same?
re: Debian, no (and it pains me to say so), I don't think they work on pure Debian, but that has more to do with our current need to pin to certain older versions than the pip stuff. |
Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
|
I think delegating the pip installation to more experienced users is fine. Most python folks will use conda. And if they want to use pip, the requirements for installation are actually all in setup.py, and giving full instructions on how to build the requirements is quite hard (CI just failed the build because it couldn't compile parts of matplotlib, which requires a lot of stuff to work properly). The way it's set up, someone that knows a bit about how python packaging works shouldn't really need any instructions as the standard "pip install the folder with the setup.py" just does the trick if your environment is "reasonable". Scikit-learn had installation instructions on how to install the build-requirements for blas across different OSs for a long time in the main installation page, and it caused people a lot of pain when they could have just "conda install"ed or even "pip install"ed. The main reason the pip install isn't that smooth for us right now is the pinning, I think, and potentially the absence of binary wheels of pyodbc for some OSs. |
|
ohh it's failing because you're parsing the environment.yml and installing it with pip which obviously fails for conda packages... so I'm not entirely sure what to do about that. Maybe I'll hard-code a fix in the conda-to-pip converter? Might be better to have a separate requirements for building the docs and using that if we want to use pip there? |
|
closes #31. |
|
Do we have an issue to move the CI to conda? |
| @@ -29,7 +29,9 @@ | |||
| pattern=r'([^=])=([^=])', | |||
| repl='\\1==\\2', | |||
| string=item) | |||
| new_dependencies.append(new_item) | |||
| if "conda" not in new_item and not new_item.startswith("python"): | |||
There was a problem hiding this comment.
Was that not handled with the excluded_dependencies below? Oh, I see, it's now no longer an exact match. Maybe move it down there just to consolidate?
Honestly this whole script hack can probably just go away if we decide that we're consolidating on some form of conda, but let's leave that for a future PR.
There was a problem hiding this comment.
do you want me to consolidate or leave it for moving to conda?
There was a problem hiding this comment.
My suggestion for now is to just switch to something like this below for now:
- if item in excluded_dependencies:
+ dep_name = re.split('[><=]+', item)[0]
+ if dep_name in excluded_dependencies:(tested locally, seems to work as expected)
We do now :) #83 |
cc @bpkroth @markusweimer
Not sure if I'm undoing some of the work @markusweimer has been doing here before.
I grouped the OS independent steps in the beginning.
I think the "manual python" version is not that useful because it's ubuntu only (not sure if the ppas work for debian?), and there's really no reason to compile your own version of pyodbc, so I removed it.