search

biocore / American-Gut

American Gut open-access data and IPython notebooks
Other
108 stars 81 forks source link

Latex formatting #3

Closed adamrp closed 10 years ago

adamrp commented 10 years ago

A few files used in the generation of the Latex templates. It is assumed that the directory structure that we are working with has one directory per sample.

ElDeveloper commented 10 years ago

http://globalnerdy.com/wordpress/wp-content/uploads/2008/03/lack_of_tests.jpg

adamrp commented 10 years ago

I also forgot the credits and license stuff... hold please....

ElDeveloper commented 10 years ago

Cool, will wait then.

On Oct 22, 2013, at 1:46 PM, adamrp notifications@github.com wrote:

I also forgot the credits and license stuff... hold please....

— Reply to this email directly or view it on GitHub.

adamrp commented 10 years ago

I can think of other ways to do the format_file bit (do_replacements) that would get around the need for the tests/support_files directory, if that's something we'd rather do. The code does, however, work as is.

ElDeveloper commented 10 years ago

Having support_files is not all that bad so I don't see a problem with that. Have you explored StringIO?

adamrp commented 10 years ago

Yeah, but I don't think you can call "open" on a StringIO object, which is something that the do_replacements function does. I could eliminate the do_replacements function altogether and put all that code in the main block in the script, but then there would be no easy way to unit test it.... the way it is, it's not even generating any files in the support_files directory, so it's not a mess and there's not even any need for tearDown in the TestCase subclass. The more I think about it, the more I think it is okay as is.

ElDeveloper commented 10 years ago

I don't think we want to remove that function specially just to test it; as more things are added to "formatting" functions/methods it becomes increasingly harder to make sure it all works fine. So support_files should be fine.

On Oct 23, 2013, at 9:28 AM, adamrp notifications@github.com wrote:

Yeah, but I don't think you can call "open" on a StringIO object, which is something that the do_replacements function does. I could eliminate the do_replacements function altogether and put all that code in the main block in the script, but then there would be no easy way to unit test it.... the way it is, it's not even generating any files in the support_files directory, so it's not a mess and there's not even any need for tearDown in the TestCase subclass. The more I think about it, the more I think it is okay as is.

— Reply to this email directly or view it on GitHub.

wasade commented 10 years ago

Ya, no open/close on stringio. Can't easily move file open/close calls to main, or contain them somehow?

On Wednesday, October 23, 2013, adamrp wrote:

Yeah, but I don't think you can call "open" on a StringIO object, which is something that the do_replacements function does. I could eliminate the do_replacements function altogether and put all that code in the _main_block in the script, but then there would be no easy way to unit test it.... the way it is, it's not even generating any files in the support_files directory, so it's not a mess and there's not even any need for tearDown in the TestCase subclass. The more I think about it, the more I think it is okay as is.

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/American-Gut/pull/3#issuecomment-26914843 .

adamrp commented 10 years ago

I found out while testing for open that you actually can do close, although there is probably seldom need to do so :)

I could move the file opens and closes to the main block, but then the do_replacements function is literally just doing string replacements, which would obviate the need for its existence, which would obviate the need for test code... but then it would have no test code. Maybe that's okay, though...

ElDeveloper commented 10 years ago

I found this problem with emperor.format at a moment. However later on I realized that the key there is to make sure that what you write in the tests is how you want it to look i. e. you take that string and render it into latex and make sure it all looks kosher. Hence if someone wanted to modify something down the road and in turn making it look not the way you initially intended then you have a frame of reference to see when this happens (the failing test).

On Oct 23, 2013, at 10:09 AM, adamrp notifications@github.com wrote:

I found out while testing for open that you actually can do close, although there is probably seldom need to do so :)

I could move the file opens and closes to the main block, but then the do_replacements function is literally just doing string replacements, which would obviate the need for its existence, which would obviate the need for test code... but then it would have no test code. Maybe that's okay, though...

— Reply to this email directly or view it on GitHub.

adamrp commented 10 years ago

I think I see what you're saying, but I don't know how easily I can check whether or not something looks like valid latex other than to use a system call to pdflatex to test it. I could do that, of course.

ElDeveloper commented 10 years ago

Hmmm, what i would do is just run the function in the test suite, print the output of the function, save it to my clipboard and put it in latex to make sure it formatted correctly.

On Oct 23, 2013, at 10:19 AM, adamrp notifications@github.com wrote:

I think I see what you're saying, but I don't know how easily I can check whether or not something looks like valid latex other than to use a system call to pdflatex to test it. I could do that, of course.

— Reply to this email directly or view it on GitHub.

adamrp commented 10 years ago

Hmm I see. Well, after discussing it with you guys, I guess I'd rather leave it as is, unless there are strong objections.

wasade commented 10 years ago

Ill defer to your judgement

On Wednesday, October 23, 2013, adamrp wrote:

Hmm I see. Well, after discussing it with you guys, I guess I'd rather leave it as is, unless there are strong objections.

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/American-Gut/pull/3#issuecomment-26920287 .

wasade commented 10 years ago

please make both scripts +x

ElDeveloper commented 10 years ago

Should we close this pull request or was this code pulled down and used?

adamrp commented 10 years ago

I think this can be closed, afaik

On Tue, Nov 12, 2013 at 11:32 AM, Yoshiki Vázquez Baeza < notifications@github.com> wrote:

Should we close this pull request or was this code pulled down and used?

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/American-Gut/pull/3#issuecomment-28319825 .

ElDeveloper commented 10 years ago

:eyes: :tongue: