-
Notifications
You must be signed in to change notification settings - Fork 84
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: Allow skipping validation when constructing workspaces #1710
feat: Allow skipping validation when constructing workspaces #1710
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1710 +/- ##
=======================================
Coverage 98.12% 98.12%
=======================================
Files 64 64
Lines 4269 4270 +1
Branches 682 683 +1
=======================================
+ Hits 4189 4190 +1
Misses 46 46
Partials 34 34
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Relevant RTD builds: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @alexander-held. This all is looking good to me and your plan for docstrings sounds great. 👍
Okay this one can get rebased and refactored now. 👍 |
fb1197f
to
4d7e86b
Compare
Congrats on the 1k'th commit @alexander-held :) |
* Add milestone to README for 1000 project commits. - 1000th commit came from Alexander Held in PR #1710
Description
This adds a
validate
kwarg to theWorkspace
constructor and toWorkspace.build
, replicating the existing functionality inModel
to skip validation against the JSON schema. For complex workspaces / models, where validation can take an appreciable amount of time (see #1699), this can be a useful option if users are sure that their inputs follow the required format.There are two things in here that are optional, I tried to split them across commits to easily take out anything you may not want:
validate
kwargs,There was previously no documentation for
validate
in theModel
constructor, so this has also been added.A test for workspace construction without validation is also included, for both construction from a specification and from model + data. This uses the same
simplemodels
setup also used elsewhere in the workspace tests,so this could be refactored if #1709 is merged first (or otherwise could be refactored in #1709). Since #1709 also adds a test at the end, merging one probably results in a conflict with the other PR as well.-> #1709 was merged before, and the implementation here was changed to re-use the fixture added in that PR.resolves #1699
Checklist Before Requesting Reviewer
Before Merging
For the PR Assignees: