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

Add documentation on transpiler plugins #781

Merged
merged 27 commits into from
Feb 27, 2024
Merged

Conversation

kevinsung
Copy link
Collaborator

@kevinsung kevinsung commented Feb 7, 2024

Fixes #102 .

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

From a quick read over the content it looks good. Just a few inline comments/suggestions.

docs/transpile/create-a-transpiler-plugin.ipynb Outdated Show resolved Hide resolved
docs/transpile/transpiler-plugins.ipynb Outdated Show resolved Hide resolved
docs/transpile/transpiler-plugins.ipynb Outdated Show resolved Hide resolved
docs/transpile/transpiler-plugins.ipynb Show resolved Hide resolved
Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

This is really nice, thanks! I followed this successfully and I understand how I'd modify the examples with my own custom code.

I just have a couple of suggestions to smooth over the user experience.

docs/transpile/create-a-transpiler-plugin.ipynb Outdated Show resolved Hide resolved
docs/transpile/create-a-transpiler-plugin.ipynb Outdated Show resolved Hide resolved
Copy link
Collaborator

@nbronn nbronn left a comment

Choose a reason for hiding this comment

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

This looks pretty great! I just wanted to include more specific examples in cases mostly, somewhat in anticipation of how users will consume this.

docs/transpile/transpiler-plugins.ipynb Outdated Show resolved Hide resolved
docs/transpile/transpiler-plugins.ipynb Show resolved Hide resolved
docs/transpile/transpiler-plugins.ipynb Outdated Show resolved Hide resolved
docs/transpile/transpiler-plugins.ipynb Show resolved Hide resolved
docs/transpile/create-a-transpiler-plugin.ipynb Outdated Show resolved Hide resolved
@kevinsung kevinsung marked this pull request as ready for review February 19, 2024 20:51
@kevinsung
Copy link
Collaborator Author

@nbronn please take another look

Copy link
Collaborator

@nbronn nbronn left a comment

Choose a reason for hiding this comment

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

LGTM

docs/transpile/transpiler-plugins.ipynb Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really nice intro! Very clear and celebrates the open-source community 👏

Copy link
Collaborator

Choose a reason for hiding this comment

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

We edit the pyproject.toml, setup.cfg, or setup.py file of our package

I'm a bit confused about this section. How does a user know which file they should update? Does it depend on what type of plugin they're developing? Whatever the reason I think it would be good to highlight how a user decides which to update

Copy link
Collaborator

Choose a reason for hiding this comment

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

Of course, your own plugin will do something more interesting than that.

Lol such sass I love it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's create our UnitarySynthesisPlugin subclass

For the code snippet following this line can you add some code comments briefly explaining each property?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add a "next steps" section to the end of the file? every docs page should have a next steps that includes at least one link to the learning platform

Copy link
Collaborator

Choose a reason for hiding this comment

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

question: do you think we should split these 3 examples up into separate pages? The page feels quite long and I wonder if it would be more digestible if split up 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm a bit confused about this section. How does a user know which file they should update? Does it depend on what type of plugin they're developing? Whatever the reason I think it would be good to highlight how a user decides which to update

pyproject.toml, setup.cfg, or setup.py are three different formats for storing the metadata for a Python project. Typically the creator of the package chooses only one of these formats, so there wouldn't be any confusion as to which file to edit (because only one of them exists). I've added a parentheses nothing this fact.

For the code snippet following this line can you add some code comments briefly explaining each property?

Done.

Could you please add a "next steps" section to the end of the file? every docs page should have a next steps that includes at least one link to the learning platform

Do you actually mean learning platform, or just the IQP platform in general (including documentation)? In any case, I couldn't think of an appropriate next step on these platforms, but I did add a next step for contributing the plugin to the Qiskit Ecosystem. Do you have any other specific pages in mind?

question: do you think we should split these 3 examples up into separate pages? The page feels quite long and I wonder if it would be more digestible if split up 🤔

I actually think it's better to have them all on one page. The right sidebar has the table of contents which makes the page easier to navigate.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you actually mean learning platform,

Yes I actually mean learning platform. In general we need to be more closely linking these 2 platforms so one of our goals is to have every docs page link to learning and vice versa. There might not be a specific page that fits perfectly for this but anything you think generally might be useful and focused around the theme of transpiling is fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added generic links to the learning platform tutorials.

Copy link
Collaborator

@javabster javabster left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@kevinsung kevinsung added this pull request to the merge queue Feb 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 27, 2024
@kevinsung kevinsung added this pull request to the merge queue Feb 27, 2024
Merged via the queue into main with commit 9c2cca9 Feb 27, 2024
5 checks passed
@kevinsung kevinsung deleted the kjs/transpiler-plugins branch February 27, 2024 16:18
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[ NEW PAGE ] Transpiler plugins
5 participants