biocore / wetlab-assistant

BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

added some simple Python code #3

Closed qiyunzhu closed 8 years ago

qiyunzhu commented 8 years ago

I have extracted a basic part of the actual Python code I wrote. The only function is to accept command line arguments (three files) and to validate them. I didn't use "click" because I wanted to free the wet lab users from installing any dependencies (including pip and conda). This PR is accumulative from the last PR (#2). PS: Travis didn't fire. I am looking into it...

mortonjt commented 8 years ago

Minor comments just for reorganization.

I'd prefer to move the command line arguments over to argparse because

I've added travis, so it should be booted in the next commit.

qiyunzhu commented 8 years ago

Hi @mortonjt , I used argparse and it worked great! I also tried to move test sample files from test_plate_mapper.py to independent files, but the test failed due to file path issues (FileNotFoundError: [Errno 2] No such file or directory: './data/input.tsv').

How about we just use the original plan (implementing test file contents in the Python code)? This was what we did in the WGS-HGT project (see here and here for examples).

Now that argument parsing became so simple, I appended the entire function to this PR. @mortonjt @antgonza

mortonjt commented 8 years ago

Awesome! The argparse really simplified the code right?

For the mean time I think this is ok. The more permanent solution is to use something like get_data_path

But I'm ok with leaving this as an issue.

Also, it looks like that your code could be refactored using pandas dataframes

But given the time constraints - I think this is ok for the time being. But we'll probably want to enforce using pandas for future iterations.

mortonjt commented 8 years ago

@sjanssen2 would you like to sanity check this and merge?

qiyunzhu commented 8 years ago

@mortonjt Thanks for your comments! argparse does a great job! I will be eager to learn pandas, numpy, scipy etc. For now, one consideration of mine is that I try to free the user (some of them are running Windows) from installing any additional modules. Therefore I did the most primitive way... Thanks for inspiring me!

mortonjt commented 8 years ago

None of those packages have windows issues ... so those should be ok for future iterations.

josenavas commented 8 years ago

I noticed that this is not following the usual repository structure of scripts and library code, and, as @mortonjt noted, is reimplementing stuff that is available on libraries. I thought that most of the people in the wet lab also had Macs?

I'm not too worry about this repository though, it should die in the following weeks since all the functionality should go into the actual plate mapper in labadmin...

qiyunzhu commented 8 years ago

@josenavas Thanks for looking at the code! I am in the process of learning how to structure a repository and how to properly choose libraries. Your comment is highly appreciated!

sjanssen2 commented 8 years ago

I'd like to see this code in action. Thus, @qiyunzhu could you please add one or more example input files as a separate PR?

qiyunzhu commented 8 years ago

@sjanssen2 Sure! Will do.

qiyunzhu commented 8 years ago

@antgonza As you noticed, there isn't extensive consideration for handling "special" cases. For example, if the user enters "BB" instead of "B" in the row head, the script will not raise exception but simply proceed and print "BB". Honestly, the spreadsheet is a very loosen format, in our perspective. The guarantee against this kind of scenario is that the wet lab start with a template, in which these headers and field names are already filled. As long as they don't accidentally modify these cells, they should be fine.

antgonza commented 8 years ago

OK, if we know the headers in advance will it be possible to have a quick check that they match. Accidents due happen. If you think this is not necessary just let me know.

qiyunzhu commented 8 years ago

@sjanssen2 examples added in PR #6.

qiyunzhu commented 8 years ago

@antgonza I think it is definitely feasible. Let me add this code then.

qiyunzhu commented 8 years ago

@antgonza I added codes to handle column and row header errors, and codes to unit-test these exceptions.