Repository navigation
Propose removing incompatibility comments regarding python2 - #8377
kakirastern wants to merge 7 commits into
Conversation
To adhere to the Fortran tradition instead, and to clear some confusion with manipulating a FITS file arising from the definition of the `origin`. Also added a comment regarding the implicit ordering of the pixel coordinate pair.
Propose to get rid of the notes regarding incompatibility of the python2 method with python3, along with the method for python2, which is no longer supported. Added in the method using pip as ``` pip install -e . # Where pip is pip3; note the dot after a space is necessary ```
|
Sorry about the additional commits I made for a closed PR earlier... Please just ignore them for now. That has to do with another issue. |
|
|
||
| or: | ||
|
|
||
| python setup.py install # Where python is python3 |
There was a problem hiding this comment.
develop is still preferred here instead of install, and it works well with python3 for a while now.
There was a problem hiding this comment.
Yup, reverted back to previous state, plus added some comments.
There was a problem hiding this comment.
well, the rest of the changes were good, my only point was that it should be "python setup.py develop" with python3 and the rest of the notes (and the mention of python setup.py install) can be taken away just as you did previously.
There was a problem hiding this comment.
But, looking at https://github.com/astropy/astropy/pull/8289/files again, it seems that you're working on an old version and this all has been reworked already.
There was a problem hiding this comment.
Yeah, I noticed that too not long ago today. But I definitely feel that I should add in a line or two regarding aliasing for pip=pip3. Give me some time and see how I could salvage the situation. Am having some mild problems setting up the development version of Astropy on my own machine... I should have it up and running over the next few days.
There was a problem hiding this comment.
do you have a recent enough version of pip?
python setup.py install is not sufficient as it installs one snapshot that will not track the changes in your local dev version, thus we had python setup.py develop there. It didn't use to work with python3, but has been fixed since.
cc @astrofrog for the pip install question.
There was a problem hiding this comment.
@kakirastern - just to check, what do you mean when you say pip install -e . doesn't work?
There was a problem hiding this comment.
@astrofrog Turned out I was missing some important dependency (cython) in addition to the jinja2 I needed to run pip install -e .[all,test] successfully. The problem has been fixed just now. I am staring at the relevant Astropy documentation of the installation of source now and realize my error... See http://docs.astropy.org/en/stable/install.html#building-from-source for my reference. I guess for a rookie like me it takes time to navigate the documentation to get the relevant information.
There was a problem hiding this comment.
Is it a good idea to include the need to check for dependencies like cython and jinja2 when using the command pip install -e .[all,test] in the section "'Activate' the development version of astropy" which is located at http://docs.astropy.org/en/latest/development/workflow/get_devel_version.html#activate-the-development-version-of-astropy?
This is just to serve as a reminder that this way of using pip to install the development version of AstroPy is effectively to "build from source", just in case? I think I would have appreciated this if some notes regarding this were in this section as well earlier. (And maybe also that these two dependencies needed to be installed with the proper pip version and not with conda even though one is working in a virtual python environment?)
There was a problem hiding this comment.
The way to tell whether it would be good to include is your experience - so, yes, do include it! I see this section very much as aimed at people who have not contributed to another project before.
|
@kakirastern - This needs some work on the git front. The branch is not originated from a recent enough astropy master, thus you have the conflict and there are unrelated commits here. So the solution would be to rebase and squash all the commits into a single one. There are some guidance here: http://docs.astropy.org/en/latest/development/workflow/development_workflow.html |
|
Thanks @bsipocz, will work on my rebasing and squashing all the commits into a single one for the same PR next. |
|
@kakirastern - thanks for looking at this carefully. Any problems you find getting up and running with the development version are of course more than fair game for another PR to add documentation! We definitely like to get rid of things that "did not use to work but now do"... |
To follow up on Issue #7287 and also Issue #8289.