pygraphistry icon indicating copy to clipboard operation
pygraphistry copied to clipboard

Dev/add dataset validation, including those for encodings

Open aucahuasi opened this issue 1 year ago • 4 comments

This PR validates the edge encodings format, checking that the attribute for the complex encoding must exists as part of the edge schema.

aucahuasi avatar Jan 31 '24 07:01 aucahuasi

awesome --

  • CI seems to fail?
  • Are there tests we can port in?
  • FEP would need to be able to call w/out passing in column names, can we have an entrypoint that still supports that?

lmeyerov avatar Feb 01 '24 04:02 lmeyerov

  • CI seems to fail?

I've fixed most of the errors; however, it seems there are some that are unrelated to the changes in this PR.

  • Are there tests we can port in?

Good catch, I ported the tests from FEP and added a couple (test_validate_encodings_with_attributes_bad and test_validate_encodings_with_attributes_good)

  • FEP would need to be able to call w/out passing in column names, can we have an entrypoint that still supports that?

Good point, I've converted the function signatures to support an empty list of attributes, and the validation only applies when the attribute list is non-empty.

aucahuasi avatar Feb 01 '24 06:02 aucahuasi

@aucahuasi on that note... CHANGELOG :)

lmeyerov avatar Feb 02 '24 22:02 lmeyerov

@aucahuasi on that note... CHANGELOG :)

I've updated the changelog, thanks!

aucahuasi avatar Feb 03 '24 01:02 aucahuasi

https://github.com/graphistry/pygraphistry/pull/548 is landing, may help w/ CI fails

lmeyerov avatar Feb 25 '24 00:02 lmeyerov

#548 is landing, may help w/ CI fails

Awesome, I've just update this PR with those changes.

aucahuasi avatar Feb 25 '24 01:02 aucahuasi

LGTM!

Want to try going through the release flow in the develop.md?

lmeyerov avatar Feb 25 '24 04:02 lmeyerov

Want to try going through the release flow in the develop.md?

I'll check the release flow, I'll comment here in case I have questions, thanks for the review!

aucahuasi avatar Feb 25 '24 05:02 aucahuasi