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

Fix custom object comparisons, allow for custom uninstantiated class encoding, fix float comparisons, allow for ignoring metadata #96

Merged
merged 42 commits into from
Apr 11, 2022

Conversation

tahoeschrader
Copy link
Contributor

@tahoeschrader tahoeschrader commented Mar 21, 2022

Describe the Changes

  • Added support for serializing, deserialization, encoding, decoding, and comparing the Unit class which comes from the pint package.
  • Doing so added a layer of complexity to the metadata matching functionality where the previous code base attempted comparing a dictionary of objects to a dictionary of objects that have been decomposed into their components. Thus, the second dictionary must have its values instantiated into objects for the comparison to work.
  • Fixed comparison between float and float64
  • Added TODO and potential code (commented out) to fix file status tracking when using pytest with norecursedirs
  • Added capability to ignore certain metadata arguments straight from the assert_match method

Example Code
No new additions have been made to the API.

Additional Notes
Objects with a to_dict() and from_dict() method no longer follow the behavior of similar objects with only a dict or slots method.

Objects of type pint.unit.Unit are assumed to be the same as objects of type pint.unit.build_unit_class..Unit.

Specific arguments in the metadata can be skipped by adding an ignore=["arg", "names", "to", "ignore"] input to assert_match.

Future Work
Figure out what to do about custom units defined by users that aren't in base pint. Object checking is currently string-based because of this.

Clean up spaghetti code I wrote for this PR.

Figure out what to do about comparing numpy arrays to lists and floats to float64. I added in hacky stuff to get that to work but there must be a better way.

Fix bug where snappiershot reports tests unchecked when those tests are part of a norecursedirs command in pytest.

@tahoeschrader tahoeschrader marked this pull request as draft March 21, 2022 22:41
@tahoeschrader tahoeschrader marked this pull request as ready for review March 24, 2022 18:35
@tahoeschrader tahoeschrader linked an issue Mar 24, 2022 that may be closed by this pull request
@tahoeschrader tahoeschrader changed the title Add custom types and fix object matching Fix custom object comparisons, allow for custom uninstantiated class encoding, fix float comparisons, allow for ignoring metadata Mar 24, 2022
Copy link

@panzer panzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot here! Nice work, excited to see this go into use!

@@ -28,6 +28,7 @@ importlib-metadata = { version = ">1.5.1", python = "<3.8" }
tomlkit = "^0.7.0"
pandas = { version = ">=0.20.0", optional = true}
pprint_ordered_sets = "^1.0.0"
pint = "^0.14"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(future work) could we rework our dependencies such that we do not impose new requirements on a "customer" project? For example, if the project using snappiershot does not use pint, we shouldn't make them install it just for snappiershot, since they won't be snapshotting pint objects if they don't use it in their project

@tahoeschrader tahoeschrader self-assigned this Mar 30, 2022
@ccheung39
Copy link
Collaborator

Great feature adds!

Copy link
Collaborator

@ccheung39 ccheung39 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve on condition of addressing last comments. Please post the future work items in either Issues/Discussions if not there already, so those tips don't get buried. Looking forward to seeing this in action in our internal Morse codebase.

@Conniemac
Copy link
Contributor

Thank you for taking this on. I will make tickets for the future work you mentioned.

@Conniemac Conniemac merged commit a413063 into develop Apr 11, 2022
@Conniemac Conniemac deleted the add-custom-types-and-fix-object-matching branch April 11, 2022 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants