Skip to content
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

Transferred over new schemas #434

Merged
merged 8 commits into from May 11, 2018

Conversation

Projects
None yet
5 participants
@dcmckayibm
Copy link
Contributor

commented Apr 28, 2018

Removed the old schemas from qiskit and transferred in the new schemas as discussed in the IBM git

Description

New schemas as represented by the OpenQuantum Specification. Also added a test script for validating the schemas against the examples.

Motivation and Context

Extend and improve the old schemas. Add new functionality for the eventual inclusion of OpenPulse.

How Has This Been Tested?

Example JSON files are given in /examples/ and examples are validated against the schemas using jsonschema using test_schemas.py

Current qiskit does not validate against the schemas (yet), so this shouldn't break anything.

Screenshots (if appropriate):

Types of changes

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

Checklist:

  • [ x] My code follows the code style of this project.
  • [x ] My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • [x ] I have added tests to cover my changes.
  • [x ] All new and existing tests passed.
@dcmckayibm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2018

It looks like in test_backend.py there is some schema checking already. What's the best course of action, try and bring everything up to the new schema standards?

@dcmckayibm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2018

I rolled back deleting the old schemas and instead moved them to an "old" directory. The tests still run against those, and we should slowly migrate from those to the new schemas, but it will be a more involved process.

@@ -0,0 +1,91 @@
# -*- coding: utf-8 -*-

This comment has been minimized.

Copy link
@diego-plan9

diego-plan9 May 7, 2018

Member

Can you move this file to tools/, adjusting the path to the schemas used inside it accordingly?

This comment has been minimized.

Copy link
@dcmckayibm

dcmckayibm May 7, 2018

Author Contributor

Does it make more sense to just make this part of the test scripts?

This comment has been minimized.

Copy link
@diego-plan9

diego-plan9 May 7, 2018

Member

Sure, test/schemas/something.py (or similar) is also an option - as long as it is not part of qiskit/* (since that folder is meant to be the folder where the source code for the SDK packages lives, and having .py files that are not part of it might cause issues with the tooling), and also not part of test/python (since currently the file does not follow unittest conventions), sounds fine!

@@ -100,7 +100,7 @@ def test_local_backend_status(self):
backend = local_provider.get_backend(name='local_qasm_simulator')
status = backend.status
schema_path = self._get_resource_path(
'backends/backend_status_schema_py.json', path=Path.SCHEMAS)
'old/backends/backend_status_schema_py.json', path=Path.SCHEMAS)

This comment has been minimized.

Copy link
@diego-plan9

diego-plan9 May 7, 2018

Member

What about using v0.1/backends/.... instead of old/backends/...? This would allow us to introduce the concept of schema versions (even if in a primitive form) and play nicer with the future changes.

This comment has been minimized.

Copy link
@dcmckayibm

dcmckayibm May 7, 2018

Author Contributor

As long as you think it will be enough to indicate that we are deprecating these schemas

@diego-plan9 diego-plan9 added this to the 0.5.0 milestone May 7, 2018

@nonhermitian

This comment has been minimized.

Copy link
Contributor

commented May 8, 2018

The backend_props_schema has the key "lastUpdateDate" that is camel cased rather than snake cased like other keys.

@dcmckayibm dcmckayibm force-pushed the dcmckayibm:master branch from 4e488a6 to 7ad9c1f May 8, 2018

@dcmckayibm

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

@nonhermitian this commit addresses the changes to the spec document and the lastUpdateDate error. @diego-plan9 this commit moves the test script to test\schemas\check_schemas.py

@diego-plan9 diego-plan9 removed this from the 0.5.0 milestone May 8, 2018

@dcmckayibm dcmckayibm force-pushed the dcmckayibm:master branch from 09bca29 to 71d7fca May 10, 2018

@dcmckayibm

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2018

I changed old to deprecated as agreed to by @jaygambetta

Updated test_backends for the change. I think this should be ready to merge.

@diego-plan9 diego-plan9 added this to the 0.5.0 milestone May 11, 2018

@diego-plan9

This comment has been minimized.

Copy link
Member

commented May 11, 2018

Thanks @dcmckayibm !

@diego-plan9 diego-plan9 merged commit 573dab7 into Qiskit:master May 11, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@diego-plan9 diego-plan9 referenced this pull request Jun 6, 2018

Closed

Introduce and use QObj #541

6 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.