DOC: Add warning for newbs not to edit auto-generated file #10456
Conversation
|
This is where That said, I think the warning in the generated file serves this purpose more directly. Warnings in docs tend to get ignored anyways :). |
|
@shoyer, hmmm.. maybe I'm confused. So are you saying it would be good to have a comment in If that's the case, we should probably have a similar comment in all the [generated] HTML files, right? |
|
@Garrett-R No, I think @jreback was thinking you could have the warning "generated.pyx is generated from make_generated.py" in the internals documentation. None of the documentation HTML pages are checked into git, so there's no danger we could get a pull request editing them directly. HOWEVER, I actually disagree with @jreback on this one. I think the warning you have here is sufficient. |
|
Ah, I see. Yeah, I agree with you. I think having the source code warning in |
|
no the issue is that I don't think it's obvious that you need to Python generate_code.py that instruction should live maybe in the contributing docs as well as in generated.pyx |
Garrett-R
force-pushed
the
warnings_for_newbs
branch
2 times, most recently
from
7dae4a6 to
dc8185d
Compare
|
Oh yeah, that's true, it's definitely not obvious that you need to run The only situation you'd ever need to know this is if you're actually trying to modify |
Garrett-R
force-pushed
the
warnings_for_newbs
branch
from
dc8185d to
cf4c3d8
Compare
| during building. To regenerate `generated.pyx`, just run: | ||
|
|
||
| `python2 generate_code.py`. | ||
|
|
There was a problem hiding this comment.
You sure? Some systems now have python pointing to python3 (ex: Arch Linux) whereas I believe all systems recognize python2... and this file is not Python 3 compatible (which tripped me up a bit)...
There was a problem hiding this comment.
+1 on python2.
On Sat, Jun 27, 2015 at 5:48 PM, Garrett Reynolds [email protected]
wrote:
In pandas/src/generate_code.py
#10456 (comment):@@ -1,3 +1,10 @@
+"""This file generatesgenerated.pyxwhich is then included in../algos.pyx
+during building. To regenerategenerated.pyx, just run:
+
python2 generate_code.py.
You sure? Some systems now have python pointing to python3 (ex: Arch
Linux) whereas I believe all systems recognize python2... and this file
is not Python 3 compatible...—
Reply to this email directly or view it on GitHub
https://github.com/pydata/pandas/pull/10456/files#r33417787.
|
hmm this should be compat but I guess that's another issue ok then on the py2 |
|
Yeah, it would be nice to have it compatible. |
| @@ -1,3 +1,10 @@ | |||
| """This file generates `generated.pyx` which is then included in `../algos.pyx` | |||
| during building. To regenerate `generated.pyx`, just run: | |||
There was a problem hiding this comment.
this actually has to be run from pandas/src (so that generated.pyx is in the correct place), so either we need to fix this (e.g. the output path should be relative to the pandas tree), or provide this in the instructions.
There was a problem hiding this comment.
Right, I fixed it so it can now be run from anywhere.
Garrett-R
force-pushed
the
warnings_for_newbs
branch
from
16976ab to
bde1e3a
Compare
| # or we get a bootstrapping problem | ||
| from StringIO import StringIO | ||
| import os | ||
| from pandas.compat import StringIO |
There was a problem hiding this comment.
Ok, I made it 2/3 compatible.
I didn't understand this warning here and the original commit had no more information about this. I tested this on Py2 and Py3 and it seemed to work fine...
Previously you had to run it from inside the same directory, but it's less likely to mess up newbs if you can run it from anywhere.
Garrett-R
force-pushed
the
warnings_for_newbs
branch
from
bde1e3a to
03bc0a1
Compare
|
merged via 758ca05 thanks! |
@jreback, here is the proposed changed we talked about in this PR for other fellow newbs that make the same mistake I did.
You said "and more importantly prob a note in
internals.rstabout how/what to change", but I didn't understand that.internals.rstis not auto-generated, right? So, why would I add a note there? Perhaps you wanted a note to appear ininternals.html? That seems unnecessary since if the user has that file on their machine, they must have intentionally generated it...