Skip to content

Possible fix for #1112 - #1113

Closed
astrofrog wants to merge 1 commit into
astropy:masterfrom
astrofrog:fix-1112
Closed

astrofrog wants to merge 1 commit into
astropy:masterfrom
astrofrog:fix-1112

Conversation

@astrofrog

Copy link
Copy Markdown
Member

Ensures that the temporary file created by PLY gets written to a temporary location, not the Astropy source tree.

@embray

embray commented May 19, 2013

Copy link
Copy Markdown
Member

This is actually a rather inefficient approach to fixing the problem. The purpose of these generated files is to speed up imports and such by avoiding having to regenerate the parse tables. This approach says "generate the optimized files but then throw them away" which just wastes even more time.

There is an option not to generate them at all. But I'm working on a solution that goes ahead and generates the files, but keeps them in the source tree. The only problem is that we have to make sure they're generated at setup time.

@astrofrog

Copy link
Copy Markdown
Member Author

Ok cool - this was just a quick hack, so I'm sure your solution will be better :) (feel free to close this)

@embray

embray commented May 19, 2013

Copy link
Copy Markdown
Member

Well I dunno yet if my solution will really work out well either. I mean it works, but I haven't figured out yet if it will be easy to do this in such a way that the generated files get installed properly. I don't think it should be hard but I haven't gotten there yet ;)

If generating the files turns out to be too much hassle we should just disable the optimization for now (rather than leave it enabled but then throw away the files generated).

@embray

embray commented May 19, 2013

Copy link
Copy Markdown
Member

Another possibility is to just go ahead and include these files in the repository. Although they are generated they shouldn't change terribly frequently. Anyone who works on tweaking any of these parsers just needs to be aware of it and to update the files...

@embray embray closed this May 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants