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

feat(snapshots): add snapshot fixtures, remove pandas fixture #151

Merged
merged 7 commits into from
May 13, 2024

Conversation

wpbonelli
Copy link
Contributor

@wpbonelli wpbonelli commented May 7, 2024

  • add numpy array snapshot extensions and fixtures with syrupy, moved from flopy
  • remove use_pandas fixture, originally meant for use in flopy tests for pandas support but never used and probably not needed, as flopy use_pandas flags will be removed eventually

@wpbonelli wpbonelli marked this pull request as ready for review May 7, 2024 13:43
@wpbonelli wpbonelli requested a review from mwtoews May 7, 2024 13:44
@wpbonelli
Copy link
Contributor Author

@mwtoews I was mainly wondering what you thought of the optional import decision here. But I think it's probably best to keep using import_optional_dependency(), will update to do so unless you disagree

@mwtoews
Copy link
Collaborator

mwtoews commented May 9, 2024

@wpbonelli no strong opinions so far, I'm just finding time to look further at the methods. One further idea is to also support compressed array ".npz" using numpy.savez_compressed.

@wpbonelli
Copy link
Contributor Author

Thanks @mwtoews for the suggestion, savez_compressed() also allows snapshots of multiple arrays at once. I pushed a commit drafting support for multiple arrays e.g.

def my_test(multi_array_snapshot):
    ...
    arrays = {
        "head": ...
        "budget": ...
    }
    assert arrays == multi_array_snapshot

which is consistent with np.load() returning a dict for multi-array .npz files.

@wpbonelli
Copy link
Contributor Author

Looks like with npz files we get inconsistent binary encodings. Strange that it's only npz files and not npy files.

@wpbonelli
Copy link
Contributor Author

I will merge as-is so we can use this for PRT tests, but would be ideal to support snapshots of multiple arrays eventually.

@wpbonelli wpbonelli merged commit c9e445d into MODFLOW-USGS:develop May 13, 2024
17 checks passed
@wpbonelli wpbonelli deleted the snapshots branch May 13, 2024 17:11
This was referenced May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants