-
Notifications
You must be signed in to change notification settings - Fork 575
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
Rename plugins folder #726
Conversation
Codecov Report
@@ Coverage Diff @@
## master #726 +/- ##
=======================================
Coverage 95.62% 95.62%
=======================================
Files 111 111
Lines 6944 6944
=======================================
Hits 6640 6640
Misses 304 304
Continue to review full report at Codecov.
|
… into rename_plugins_folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me @mariaschuld! Thanks!
-
One important spot would be updating the
Makefile
too (here and once later):
Line 6 in 51dd8b4
PLUGIN_TESTRUNNER := -m pytest pennylane/plugins/tests --tb=native --no-flaky-report -
Going for renaming the entry points could occur right before release for example.
Great, thanks for pointing out the MakeFile! |
@josh146 I think the device tests are failing because CI needs to change the file path with this PR? |
@mariaschuld yep! You'll need to update the corresponding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mariaschuld!
You mentioned
It is part of an overall effort to clarify our device ecosystem for users and developers.
I just wanted to ask roughly what the next steps will be? I noticed "plugin" is mentioned a lot still in the docs, could there be any occurrences there that warrant changing to "device"? Maybe that is for a follow up PR.
.github/CHANGELOG.md
Outdated
@@ -64,6 +64,10 @@ | |||
|
|||
<h3>Improvements</h3> | |||
|
|||
* The `pennylane.plugins` and `pennylane.beta.plugins` folders have been renamed to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to update "Adds a device test suite, located at pennylane/plugins/tests
"? This is for the current release so think it makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also noticed that previously there are some links (line 140 and 143) that might be nice to update - I don't think we should change everything in the history, but it would be good for links to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be better to do in separate PR? I am trying to be more disciplined with such things :)
@@ -1,7 +1,7 @@ | |||
qml.plugins | |||
qml.devices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a bit late to point this out, but I suddenly wonder if we will be introducing confusion between
qml.device
qml.Device
qml.devices
I suppose only option is to be really clear in the docs
The next steps would be more about capabilities of devices that we need to standarise: Some don't have the attribute |
This PR renames the plugins folders in core PennyLane to "devices", to avoid confusion for developers (since what they contain are devices). It also updates some outdated docstrings here and there.
It is part of an overall effort to clarify our device ecosystem for users and developers.
NOTE:
pip install -e .
How can we communicate this to external developers?