adding testing check_data function to load_data

[sabinala]

This PR adds a function to check for formatting errors in a dataset within the load_data function that is called whenever sample is used. I've also included a notebook where these errors are produced.

Closes #454
Closes #290

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[sabinala]

Changing back to WIP to add: (1) check that there is a column of data corresponding to everything mentioned in the data mapping, and (2) a data report that tells the shape of the data and what columns are included

[SamWitty]

@sabinala , what is the status of this PR?

[sabinala]

@SamWitty the status of this PR is that it's still in progress. I need to convert the assert statements to the form if (condition) raise (error message), and then write tests for these errors.

[sabinala]

@djinnome let me know if you have questions on this. I added a check_data and a print_dataframe_report function inside of load_data in pyciemss/integration_utils/observation.py, and the test test_load_data to tests/test_interfaces.py which uses some "bad datasets" added to fixtures.py. Importantly, in order for the testing to work, I had to change load_data so that it can accept a DataFrame as well as a file path (string).

[sabinala]

@djinnome the only issue is that I was not able to create a dataset with "NaN" values in fixtures.py for testing, but the error will still be caught by the data checker function.

[sabinala]

@djinnome let's talk about this during our meeting later...it looks like some tests aren't passing, but the problems are coming from tests for visuals and interruptions? When I run make format and pytest tests/test_interfaces.py I get no issues/all tests passing.

[sabinala]

This PR is ready to go, but currently blocked by #481. (Still getting this issue after having merged main into this branch and running pip install -e . or pip install -e .[tests].)

[sabinala]

I'm confused as to why it appears this PR is failing on linting...when I run make format no files are changed.

[djinnome]

I think it is because local flake8 is out of sync with the flake8 on the CI. Should we upgrade local flake8 or downgrade the CI flake8 @SamWitty ?

[SamWitty]

Please update local flake8. Thanks for checking!

[sabinala]

@SamWitty @djinnome I updated local flake8 with pip install --upgrade flake8 and am still failing that test:

===================================== FAILURES ======================================
_______________ test_export_PNG[schema_file2-ref_file2-trajectories] ________________

schema_file = PosixPath('/Users/altu809/Projects/pyciemss/pyciemss/visuals/schemas/trajectories.vg.json')
ref_file = PosixPath('/Users/altu809/Projects/pyciemss/tests/visuals/reference_images/trajectories.png')
name = 'trajectories'

    @pytest.mark.parametrize("schema_file, ref_file, name", schemas(ref_ext="png"))
    def test_export_PNG(schema_file, ref_file, name):
        """
        Test all default schema files against the reference files for PNG files
    
        schema_file: default schema files saved within the visuals module
        ref_file: compare the created  png to this reference file
        name: stem name of reference file
        """
        with open(schema_file) as f:
            schema = json.load(f)
    
        image = plots.ipy_display(schema, format="PNG", dpi=72).data
        save_result(image, name, "png")
    
        test_threshold = 0.04
        JS_boolean, JS_score = png_matches(image, ref_file, test_threshold)
>       assert (
            JS_boolean
        ), f"{name}: PNG Histogram divergence: Shannon Jansen value {JS_score} > {test_threshold} "
E       AssertionError: trajectories: PNG Histogram divergence: Shannon Jansen value 0.1562242136437859 > 0.04 
E       assert False

tests/visuals/test_schemas.py:148: AssertionError

[SamWitty]

This is not a linting error. The test itself is failing. This happens sometimes, as the tests are randomized. Rerunning failing tests should work most of the time.

[sabinala]

@SamWitty correct again! All tests have passed, @djinnome this is ready now for review.

[djinnome]

Is this a constraint that we want to impose? It might be better if we could handle ragged data, yes?

[sabinala]

@djinnome It would be better, but I'm not sure where to go with that. How would that propagate to calibrate? I think it's probably best to throw an error message for now, and create a new issue to handle ragged data in the future.

[djinnome]

Approving. Perhaps we should create an issue for a feature request to relax the missing data constraint.