-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make parsetab/lextab generation more automatic/maintainable #7177
Comments
Having a written (reproducible) procedure to regenerate the files would be useful indeed, as it is not clear which imports are needed to trigger each file generation. For me a "quick and not too dirty" script would be good enough (it could remove the lextab/parsetab files and do the needed imports). |
OK, I think I've experimented enough with But the question still stands if it's desirable - that is, if we should stop packaging them with the source code at all. It's even easier to make that change than making the script actually: basically, delete the files from git and we are done - they auto-generate at import-time. And my testing says the time to generate the tables is trivial (ms), so a user likely won't notice at all. (That could change if the tables become much more complicated... but that's premature optimization.) (I guess I'm slowly convincing myself that's what we should do...) |
Is there a reason why we didn't want to rely on auto-generation originally? Performance? |
Relevant commits/issues for my question: ef839a1#diff-d33d630835d502d02afae8a94d37f7ce In particular the following comment in #1112
This is fixed now though. I guess one question is whether in e.g. root installs of Astropy the files could not be generated due to permission errors? We'd have to make sure they are generated at install time. |
Agreed that install-time (or probably build-time) is the best way to go @astrofrog. I'll assign this to myself, as I spent some time last week figuring out how all of this actually works. (Soon enough I'll be saying things like #4318 (comment) 😉) |
In #7174 the PLY lextab/parsetab tables were updated and some instructions were added on how to do this. But some of the discussion around it (in the PR and on slack) reveals that there are some complexities there. This issue is to build a plan for how we might go about making this more maintainable.
The current process is to delete the table files, re-build astropy inplace, and then run the tests in the part of the astropy code base that uses the tables. This is necessary because at least in units, the tables only get generated when the code using them is hit.
However, the table-building process is pretty fast. So some alternatives include:
extern
.python setup.py make_lexparsetabs
or something. That will require adding some code or other annotation that figures out where all the table-generation code is. This is non-trivial right now because forcoordinates
you just have to create the_AngleParser
object, but inunits
I think you actually have to generate the relevant unit (which requires importing all the various unit sub-packages).from astropy.coordinates import make_parselextabs;make_parselextabs()
or similar. Still requires most of the items from 2 above, though.cc @astrofrog @saimn @bsipocz
The text was updated successfully, but these errors were encountered: