Skip to content

Python: remove Verify and ./diagnostics - #287

Merged
Devis Lucato (dluc) merged 13 commits into
microsoft:python-previewfrom
jjhenkel:jjhenkel/remove-verify
Apr 5, 2023
Merged

Devis Lucato (dluc) merged 13 commits into
microsoft:python-previewfrom
jjhenkel:jjhenkel/remove-verify

Conversation

@jjhenkel

@jjhenkel Jordan Henkel (jjhenkel) commented Apr 3, 2023 •

Copy link
Copy Markdown

Motivation and Context

In an effort to make the Python port more idiomatic, let's remove the Verify class and the ./diagnostics sub-module. There are many more follow-on tasks to do here, but this is a good start (and already a large enough change).

Description

This PR does the following:

  1. Removes the Verify class, and re-writes all instances of Verify.*
  2. Adds a validation.py in the ./utils sub-module to hand some of the more complex cases from Verify (checking that skills/functions/and function params have valid names)
  3. Removes the rest of the ./diagnostics sub-module (w/ a longer-term goal of removing all/most of our custom exception classes and, instead, using appropriate built-in error classes)

Contribution Checklist

For Python, I've run make pre-commit to verify style/linting; I've run pytest tests/. No tests added here, this is a cosmetic/style change.

image

@jjhenkel

Copy link
Copy Markdown
Author

One more small thing, just had to merge and test_config.py was out of compliance w/ make pre-commit. Brought it back into compliance, we should re-factor and standardize how we test skills (defined as files). (But that can come in a separate PR.)

@adrianwyatt Adrian Bonar (adrianwyatt) added the python Pull requests for the Python Semantic Kernel label Apr 3, 2023
@mkarle

Copy link
Copy Markdown
Contributor

Looks good to me!

@dluc
Devis Lucato (dluc) merged commit f7c8b10 into microsoft:python-preview Apr 5, 2023
Devis Lucato (dluc) pushed a commit that referenced this pull request Apr 12, 2023
### Motivation and Context

In an effort to make the Python port more idiomatic, let's remove the
`Verify` class and the `./diagnostics` sub-module. There are many more
follow-on tasks to do here, but this is a good start (and already a
large enough change).

### Description

This PR does the following:

1. Removes the `Verify` class, and re-writes all instances of `Verify.*`
2. Adds a `validation.py` in the `./utils` sub-module to hand some of
the more complex cases from `Verify` (checking that skills/functions/and
function params have valid names)
3. Removes the rest of the `./diagnostics` sub-module (w/ a longer-term
goal of removing all/most of our custom exception classes and, instead,
using appropriate built-in error classes)
Devis Lucato (dluc) pushed a commit that referenced this pull request Apr 13, 2023
### Motivation and Context

In an effort to make the Python port more idiomatic, let's remove the
`Verify` class and the `./diagnostics` sub-module. There are many more
follow-on tasks to do here, but this is a good start (and already a
large enough change).

### Description

This PR does the following:

1. Removes the `Verify` class, and re-writes all instances of `Verify.*`
2. Adds a `validation.py` in the `./utils` sub-module to hand some of
the more complex cases from `Verify` (checking that skills/functions/and
function params have valid names)
3. Removes the rest of the `./diagnostics` sub-module (w/ a longer-term
goal of removing all/most of our custom exception classes and, instead,
using appropriate built-in error classes)
Devis Lucato (dluc) pushed a commit that referenced this pull request Apr 13, 2023
### Motivation and Context

In an effort to make the Python port more idiomatic, let's remove the
`Verify` class and the `./diagnostics` sub-module. There are many more
follow-on tasks to do here, but this is a good start (and already a
large enough change).

### Description

This PR does the following:

1. Removes the `Verify` class, and re-writes all instances of `Verify.*`
2. Adds a `validation.py` in the `./utils` sub-module to hand some of
the more complex cases from `Verify` (checking that skills/functions/and
function params have valid names)
3. Removes the rest of the `./diagnostics` sub-module (w/ a longer-term
goal of removing all/most of our custom exception classes and, instead,
using appropriate built-in error classes)
Desmond Howard (dehoward) pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
### Motivation and Context

In an effort to make the Python port more idiomatic, let's remove the
`Verify` class and the `./diagnostics` sub-module. There are many more
follow-on tasks to do here, but this is a good start (and already a
large enough change).

### Description

This PR does the following:

1. Removes the `Verify` class, and re-writes all instances of `Verify.*`
2. Adds a `validation.py` in the `./utils` sub-module to hand some of
the more complex cases from `Verify` (checking that skills/functions/and
function params have valid names)
3. Removes the rest of the `./diagnostics` sub-module (w/ a longer-term
goal of removing all/most of our custom exception classes and, instead,
using appropriate built-in error classes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants