Skip to content
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

new data pull in & validate #111

Merged
merged 5 commits into from
Dec 12, 2015
Merged

Conversation

briankleinqiu
Copy link
Contributor

Edited the data folder so that make data will pull in the raw ds005 the new func ds005, and the mni templates folder.

make validate should validate all of that with the new mni_hashlist i put in.

Also renamed mni_icbm152... folder itself to 'templates' and the mni_icbm152_t1... file we'll be using into 'mni_standard.nii' for convenience and clarity. mni_standard.nii is the one matthew mentioned as most useful for displaying the patterns on

This will prbly screw up travis ci somewhere but we'll see

@BenjaminHsieh
Copy link
Contributor

yeah...the coverage dropped like crazy, not sure how to fix... can you play around with the coverage in makefile of project directory and/or add coverage of makefile of data directory and see how travis responds? If nothing works raise an issue and ask jarrod/ross

@changsiyao
Copy link
Contributor

Are you generating a hash list every time you run data.py?
You should generate the hash list yourself and everybody should just compare their hash to the txt file you uploaded.
I don't see why you need the make_hash_list function.

@briankleinqiu
Copy link
Contributor Author

The make_hash_list function I just left in, in case I wanted to make another hash list, so it's not actually being called. I git added a prior hash list that I had generated before to compare to, yeah. I'll mess with it around some more and see with the travis ci.

@changsiyao
Copy link
Contributor

But it seems like that's messing up travis. I had to put the generate hash script in the tools folder to avoid travis problems, if you recall.

@briankleinqiu
Copy link
Contributor Author

Commented that out just now, should improve at least a little now

@changsiyao
Copy link
Contributor

What is the new_hashList.txt, what data is that for?

@briankleinqiu
Copy link
Contributor Author

new_hashList.txt is the hashlist for ds005 that includes the mni_func data, so it actually makes the original hashlist redundant for now. but should keep the old hashlist in until we finalize things more

@changsiyao
Copy link
Contributor

and mni_hashlist is for the templates?
Also, what does the templates do? I'm kinda confused.

@briankleinqiu
Copy link
Contributor Author

Yup, mni_hashlist is for the templates.

Templates is just the folder:

http://nipy.bic.berkeley.edu/rcsds/mni_icbm152_nlin_asym_09c_2mm/

I renamed mni_icbm152_nlin_asym_09c_2mm to templates for clarity and convenience so we don't have to bother with that complicated name.

That's also why I renamed mni_icbm152_t1_tal_nlin_asym_09c_2mm.nii inside that folder to mni_standard.nii. This nii file is the standard brain template we should use to display functional patterns on, according to the Readme updated by Matthew.

Also we'll probably end up using the mask template in there, but haven't renamed that yet.

As for the travis ci, i think combining all the hashlists and doing only one check_hashes might help with the remaining -2.4%

@changsiyao
Copy link
Contributor

Travis won't cover the main function, last time, we also ignored the non-coverage of the last few lines. So I suppose if we combine everything into 1 check hashes, it's going to help, but it might also be confusing to people reading our codes. I'm not so sure about if we should combine all the hash lists...

@BenjaminHsieh
Copy link
Contributor

To avoid coverage on data.py entirely:

@BenjaminHsieh
Copy link
Contributor

You can let coverage check only the functions inside by creating a new python script with only functions
Then go to the main project directory and change the make coverage line:
--cover-package=data/thenewfunction.py

@BenjaminHsieh
Copy link
Contributor

How did this go?

@briankleinqiu
Copy link
Contributor Author

Is there any other reason not to combine the hashlists? I'm not sure why it'd be confusing since the idea is to check if all the data pulled in matches our hashlist, and it shouldn't matter if there's three hashlists or one?

I combined mni_hash.txt and newhashlist.txt into total_hash.txt, so it checks for everything in ds005 and the mni templates folders. It looks like it'll pass the travis so is it okay if we go with this?

If so I'll remove all the extraneous hashlists and code and then merge the pull request

@BenjaminHsieh
Copy link
Contributor

I think the main thing is to make sure the hashes are checked in the right order as they are generated, did you try downloading the data and validating it? if it works then this should be fine

@briankleinqiu
Copy link
Contributor Author

Wait are you sure they need to be checked in a certain order? I did the make validate and that was fine.

@BenjaminHsieh
Copy link
Contributor

well, i assumed the for loop in check_hashes requires a certain order, if download and validate work fine then we can go ahead and merge

@briankleinqiu
Copy link
Contributor Author

Added a readme

briankleinqiu added a commit that referenced this pull request Dec 12, 2015
new data pull in & validate
@briankleinqiu briankleinqiu merged commit a2565ae into berkeley-stat159:dev Dec 12, 2015
@BenjaminHsieh
Copy link
Contributor

lol next time probably should wait for travis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants