pr-omethe-us / PyKED

Python interface to the ChemKED database format
https://pr-omethe-us.github.io/PyKED/
BSD 3-Clause "New" or "Revised" License
15 stars 15 forks source link

Consider allowing validation to be turned off #39

Open bryanwweber opened 7 years ago

bryanwweber commented 7 years ago

It would be very useful for quick & dirty work (particularly while the schema is changing so much) to be able to disable validation so that we can use PyKED without having to rewrite YAML files every week. Obviously all the files that go in the database have to validate properly, but this might be useful long term too.

kyleniemeyer commented 7 years ago

Yeah, definitely. I added this in as a hack by doing return True at the top of _validate_isvalid_reference(), but would be good to add a method to do this.

kyleniemeyer commented 7 years ago

Do we want to disable all validation, or just reference validation?

I suppose "all" validation just includes the methods in OurValidator: _validate_isvalid_unit(), _validate_isvalid_quantity(), _validate_isvalid_reference(), and _validate_isvalid_orcid().

kyleniemeyer commented 7 years ago

Or, do we just want to have it skip all validation, as in just skip over self.validate_yaml(properties)?

bryanwweber commented 7 years ago

I think all validation, but maybe we could make it customizable somehow

kyleniemeyer commented 7 years ago

Yeah, that's a good idea. I can imagine use cases where you want to skip reference/author validation (which requires internet access and could be slow, etc.), but still ensure required fields are present.

I've started a branch to add this, and will finish it today.

bryanwweber commented 7 years ago

Watch out though, because a lot of the validation code will change with the uncertainty branch, so doing this now might make for tougher merges later :smiley:

kyleniemeyer commented 7 years ago

Yeah yeah... I recognize that is coming from you wanting me to wrap up my uncertainty branch stuff 😋

Honestly though, I was planning on implementing all this with one or more variables like _skip_validation and just checking if that is true at the beginning of the custom validator functions... so we might need to add that in a place or two with uncertainty, but hopefully it won't be totally borked.

bryanwweber commented 7 years ago

OK sounds good :stuck_out_tongue_closed_eyes:

bryanwweber commented 7 years ago

See #49 for a partial implementation of this

kyleniemeyer commented 7 years ago

I'm fine with closing this—there may be reasons to add the ability to disable parts of the validation with more granularity, but this should be sufficient for most needs.

bryanwweber commented 7 years ago

I still think its worth having general schema validation without having DOI/ORCID checks. I actually think a more common use will be to turn off online validation but leave the rest of the validation turned on. I'm going to open this again so we remember to implement that, but it doesn't have to be high priority

kyleniemeyer commented 6 years ago

OK, so I agree at minimum that we should be able to disable validation of the reference field—it should still be proper YAML, but have Cerberus ignore it. Perhaps in keeping with the existing option, the option could be skip_reference_validation=False (by default) ?

kyleniemeyer commented 6 years ago

OK, so here's where we are now: reference with year and authors is required, but nothing else.