MobleyLab / chemper

Repository for Chemical Perception Sampling Tools
MIT License
19 stars 10 forks source link

Add nbval to track jupyter notebooks #21

Closed bannanc closed 6 years ago

bannanc commented 6 years ago

This is replacing #18 which had a ton of redundant commits from playing around with what is wrong.

I'm adding nbval package so I can test the jupyter notebooks in examples. For now I'm using the option py.test --nbval-lax which only checks that the notebook runs without error. The normal call py.test --nbval also checks that when the notebook is rerun the output doesn't change. However, I'm using dictionaries where the printed output isn't always consistent so for now I'm not checking out.

From this call you can add #NBVAL_CHECK_OUTPUT to the cells where you wish to know if the output is the same as that in the stored notebook.

bannanc commented 6 years ago

Just for the record, the cell that is timing out is this:

mol = mol_toolkit.MolFromSmiles('CC')
atom_index_list = get_smirks_dict(mol, smirks_list)
# ethane only has two matching SMIRKS patterns
print(atom_index_list.keys())

Here is the code for get_smirks_dict it is a couple of cells up

def  get_smirks_dictget_smi (mol, smirks_list):
    """
    mol - chemper Mol object
    smirks_list - list of tuples (SMIRKS, label)

    Returns a dictionary of listes
    {label: [ {smirks_index: atom_index} ] }
    """
    temp_dict = dict()
    for smirks, label in smirks_list:
        for dic in mol.smirks_search(smirks):
            atom_tuple = tuple([dic[i+1].get_index() for i in range(len(dic))])
            temp_dict[atom_tuple] = label

    label_dict = dict()
    for atom_tuple, label in temp_dict.items():
        if label not in label_dict:
            label_dict[label] = list()

        label_dict[label].append({i+1: atom_idx for i, atom_idx in enumerate(atom_tuple) })

    return label_dict

This function might not be the MOST efficient way to do what I'm trying for, but it does not take 10 minutes, its within a couple seconds when I run locally.

bannanc commented 6 years ago

@dgasmith and @janash If you get a chance, I'm at a loss as to why this isn't working, the tests run fine locally.

codecov-io commented 6 years ago

Codecov Report

Merging #21 into master will decrease coverage by 2%. The diff coverage is n/a.

bannanc commented 6 years ago

I suspect the problem is the same as #23 where the oe_license isn't accessible in the example or temporary folders for some reason. Still working on a work around for that.

nathanmlim commented 6 years ago

First have Travis install nb_conda and then

Try adding a cell that does ‘’’ Import os os.environ[“OE_LICENSE”]=“path to license” ‘’’

bannanc commented 6 years ago

OK, thanks I'll check this out tonight/tomorrow morning.

bannanc commented 6 years ago

OK, so it turns out this whole thing wasn't working because I had a separate global and env section in my travis file. The openeye module has two ways of looking for a license it will either accept a global variable (OE_LICENSE) or the file oe_license.txt in your current working directory.

The global section in my file before wasn't working because it wasn't under env so travis didn't have the global variable OE_LICENSE. Most of the tests pass because pytest runs from the home directory where the oe_license was being stored. However, the nbval tests run the notebook in its own directory (examples). Without the OE_LICENSE variable the cells wouldn't run, but for some reason they were timing out instead of throwing exceptions.

While I have #23 also passing, I think this is a more general way to handle jupyter notebook tests since we don't have to worry about the weird magic command conversions or updating the test file when we make a new example.