Skip to content

Add wigner functions#1

Merged
markjeveritt merged 1 commit into
masterfrom
markjeveritt-patch-1
Oct 25, 2017
Merged

Add wigner functions#1
markjeveritt merged 1 commit into
masterfrom
markjeveritt-patch-1

Conversation

@markjeveritt
Copy link
Copy Markdown
Owner

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

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

Checklist:

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

@markjeveritt
Copy link
Copy Markdown
Owner Author

@diego-plan9 I think I have included the new files in a new pull request as suggested.

Russell Rundle authored these and I have been unable to test that everything works OK - I have seen the Jupyter notebook working so this would probably be the best place to check that all is OK.

@diego-plan9
Copy link
Copy Markdown

Hello @markjeveritt ! I'm glad you pinged by mentioning my name with an @ ... as otherwise I might have missed the notification, since it seems this PR has been issued in your fork and should be issued in the main repository at: https://github.com/QISKit/qiskit-sdk-py/pulls. Could you please make another PR there?

Hopefully Chris will be able to retake the review of the PR and provide more detailed instructions, as it seems there are still some minor organizational changes to do in order to conform to the SDK structure, as per his comments at Qiskit#3 (comment) - the short version is that the python files should be moved to somewhere inside the qiskit/tools/<somewhere> folder, and probably the notebook moved to the tutorials repository.

@markjeveritt
Copy link
Copy Markdown
Owner Author

Hi @diego-plan9 I'm glad too! The last time we did this we were explicitly instructed to fork and then submit a PR - because we did not have permission from the main branch. Has this changed please?

I will click the merge pull request to see if this merges with the base branch (sorry if this is wrong - we are not power users of git).

We added two files to qiskit/tools/ these are wigner_function_visualisation.py and wigner_function_tomography.py. We also added a jupyter notebook to examples/jupyter/ which was wigner_functions.ipynb (plus some supporting images).

@markjeveritt markjeveritt merged commit dda7318 into master Oct 25, 2017
@diego-plan9
Copy link
Copy Markdown

Hi @markjeveritt ,

no worries at all - git and github takes some time to get used to! You do have full control over your fork (ie. over marjeveritt/qiskit-sdk-py) and all the branches on it, but indeed the workflow for the main repository (ie, qiskit/qiskit-sdk-py) is usually similar to:

  • you fork the repository into your personal account (as you did already)
  • optionally, you create a branch on your repository (again, as you did: marjeveritt/qiskit-sdk-py:markjeveritt-patch-1, for example). This is not mandatory, but helps a lot for organizing your fork when working in several issues at the same time.
  • you commit your changes to that branch on your repository.
  • once you are happy with the changes, you submit a pull request to the main repository from your branch (ie. trying to merge marjeveritt/qiskit-sdk-py:markjeveritt-patch-1 into qiskit/qiskit-sdk-py:master). This is what has changed a bit compared to your initial comment - we previously suggested issuing the PR against qiskit/qiskit-sdk-py:develop.
  • we then review the PR and usually suggest changes - if that is the case, you should make those changes in your branch marjeveritt/qiskit-sdk-py:markjeveritt-patch-1, and push them there. When you push to your branch, the PR gets auto-magically updated.
  • finally, when everything is ready, we proceed to merge the branch into the main repository.

When you merged this PR, you modified only your fork - if you check your master branch at:
https://github.com/markjeveritt/qiskit-sdk-py
You will see that it says something similar to "This branch is 3 commits ahead, 2 commits behind QISKit:master."

If you click on the "compare" link at the right of that text, you will get to a page such as:
Qiskit/qiskit@master...markjeveritt:master

Which shows the changes - and has a very handy "Create pull request" button that will create the PR in the main repository! Specifically, it uses the four dropboxes at the top for deciding which repo and branch you want to target (ie. Qiskit/qiskit, base) and the branch you want to merge from (marjeveritt/qiskit-sdk-py, your-branch - in this case it will show master, as it seems the changes were not commited into the branch, but to your master).

It also seems that the files have been duplicated in the process, as they are appearing both in the root path and in the other folders - would it be possible to remove the old ones (if using the command line, git rm file.py should do the trick) before issuing the PR?

Hope this makes sense! Again, please be aware that Chris might suggest further changes as he is more familiar with that part of the SDK, but hopefully we are on the right track :)

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