Skip to content

Document .validate_templates()#308

Merged
dekatzenel merged 5 commits intomasterfrom
documentation/validate-templates
Apr 23, 2020
Merged

Document .validate_templates()#308
dekatzenel merged 5 commits intomasterfrom
documentation/validate-templates

Conversation

@dekatzenel
Copy link
Copy Markdown
Contributor

Citrine Python PR

Description

Document .validate_templates()

PR Type:

  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Adherence to team decisions

  • I have added tests for 100% coverage
  • I have written Numpy-style docstrings for every method and class.
  • I have communicated the downstream consequences of the PR to others.
  • I have bumped the version in setup.py

@dekatzenel dekatzenel requested review from bfolie and maxhutch April 22, 2020 20:11
@bfolie
Copy link
Copy Markdown

bfolie commented Apr 22, 2020

Could you please make it one sentence per line (for ease of future PR review)?

Comment thread docs/source/data_entry.rst Outdated
Template and Simple Validations
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sometimes, it is convenient to be able to bulk validate a group of runs and/or specs against their attribute and object
templates before any of the data objects is stored. The ``.validate_templates()`` methods, available for all runs and
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
templates before any of the data objects is stored. The ``.validate_templates()`` methods, available for all runs and
templates before any of the data objects are stored. The ``.validate_templates()`` methods, available for all runs and

Comment thread docs/source/data_entry.rst Outdated
process_template = ProcessTemplate("pt")
process_spec = ProcessSpec("ps", template=process_template)
material_spec = MaterialSpec("ms", process=process_spec)
ingredient_spec = IngredientSpec("is", process=process_spec, material=material_spec)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't this creating a loop (the process makes the material which is used as an ingredient in the process)?

Comment thread docs/source/data_entry.rst Outdated
process_spec = ProcessSpec("ps", template=process_template)
material_spec = MaterialSpec("ms", process=process_spec)
ingredient_spec = IngredientSpec("is", process=process_spec, material=material_spec)
dataset.ingredient_specs.validate_templates(run, ingredient_process_template=process_template)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What's run?

Comment thread docs/source/data_entry.rst Outdated
Notably, these methods do not validate linked objects in any way, making it possible to run validations on an object
with links to yet-unstored objects. Be aware that this means that ``.validate_templates()`` does not validate links and
will not surface any link-based errors. For ingredients, the associated object template is a process template that is
provided as a separate parameter. This method returns a list of validation errors, which is empty on validation success.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The sentence about ingredients seems like a secondary set of information: once the reader understands validation in the general case, here's an extra wrinkle for the ingredients case. I suggest it be separated. After explaining and showing examples for typical validation, then explain and show an example for ingredient validation.

Comment thread docs/source/data_entry.rst Outdated
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sometimes, it is convenient to be able to bulk validate a group of runs and/or specs against their attribute and object
templates before any of the data objects is stored. The ``.validate_templates()`` methods, available for all runs and
specs, validate the provided object against already-stored attribute templates as well as a provided object template.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Where do these attribute templates come from? Are links to them expected to be in the object template?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not necessarily. Links are expected to be on the object

@bfolie
Copy link
Copy Markdown

bfolie commented Apr 22, 2020

Could you expand on the examples? They show the structure of the methods, but their usage would make more sense if some example outputs were shown. Since the successful case results in an empty list, there should be examples of failed validation.

material_spec = MaterialSpec("ms", process=mat_process_spec)

ingredient_spec = IngredientSpec("is", process=process_spec, material=material_spec, labels=["ingredient"])
dataset.ingredient_specs.validate_templates(ingredient_spec, ingredient_process_template=process_template)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If the collection is ingredient specs/runs, is the object_template field ignored?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sort of? It's passed through to the core validation code, which then validates the object template against all attributes that have attribute templates that are also on the object template. But ingredients don't have attributes, so there's nothing to validate the object template against

Comment thread docs/source/data_entry.rst Outdated

Template and Simple Validations
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sometimes, it is convenient to be able to bulk validate a group of runs and/or specs against their attribute and object
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
Sometimes, it is convenient to be able to bulk validate a group of runs and/or specs against their attribute and object
Sometimes, it is convenient to validate a group of runs and/or specs against their attribute and object

Comment thread docs/source/data_entry.rst Outdated
Sometimes, it is convenient to be able to bulk validate a group of runs and/or specs against their attribute and object
templates before any of the data objects are stored.
The ``.validate_templates()`` methods, available for all runs and specs, validate the provided object against all of the
(already-stored) attribute templates linked to attributes on the object as well as an optional provided object template.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
(already-stored) attribute templates linked to attributes on the object as well as an optional provided object template.
(already-stored) attribute templates linked to attributes on the object as well as against an optional object template.

Comment thread docs/source/data_entry.rst Outdated
Comment on lines +145 to +146
Be aware that this means that ``.validate_templates()`` does not validate links and will not surface any link-based
errors.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
Be aware that this means that ``.validate_templates()`` does not validate links and will not surface any link-based
errors.
Be aware that this means that ``.validate_templates()`` will not surface any link-based errors.

Comment thread docs/source/data_entry.rst Outdated
Comment on lines +149 to +150
The below example usages are trivial examples intended to illustrate the method arguments, rather than interesting
validation cases.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
The below example usages are trivial examples intended to illustrate the method arguments, rather than interesting
validation cases.
The examples below illustrate the usage of ``.validate_templates()``.

@bfolie
Copy link
Copy Markdown

bfolie commented Apr 23, 2020

I made stylistic suggestions. You can incorporate them as you see fit and then I'll approve

@dekatzenel dekatzenel merged commit 6372559 into master Apr 23, 2020
@dekatzenel dekatzenel deleted the documentation/validate-templates branch April 23, 2020 15:28
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