-
Notifications
You must be signed in to change notification settings - Fork 154
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 dwave.samplers and dwave.composite namespace #21
base: master
Are you sure you want to change the base?
Add dwave.samplers and dwave.composite namespace #21
Conversation
72b4a55
to
8149fa1
Compare
8149fa1
to
c2ccfd4
Compare
Add files via upload
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.
LGTM! :)
@@ -27,6 +29,8 @@ | |||
else: | |||
exec(open("./dwaveoceansdk/package_info.py").read()) | |||
|
|||
os.chdir(os.path.dirname(os.path.abspath(__file__))) |
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.
Not sure why do you need this?
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.
No idea, probably don't
from pkg_resources import iter_entry_points | ||
|
||
# add the samplers to the module from entrypoints, name conflicts override | ||
globals().update((ep.name, ep.load()) for ep in iter_entry_points('dwave.samplers')) |
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.
Oh, since now we have samplers in Hybrid as well, maybe we want to differentiate those from dimod samplers?
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.
Something like:
dwave.dimod.samplers
dwave.hybrid.samplers
Or think of a way to reconciliate the two. Same goes for dwave.composites
.
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.
how about
dwave.samplers
dwave.composites
dwave.runnables # this could be further subdivided
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 don't love it, but it could work.
On a somewhat related note, I'm getting fond of from dwave import dimod, hybrid
.
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.
Are you proposing making dimod and hybrid into namespace packages? That would be easy to do, but at some point it would make sense to just make them all into a monolith package.
Or do you mean like the above where just the samplers/composites are present? I think it would be extremely confusing for from dwave import hybrid, dimod
to contain different contents/structure than import hybrid, dimod
.
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.
The former. I'm toying with the idea.
Create a namespace for Samplers and Composites populated by entry points.
We could also consider having this as a separate package rather than part of the SDK.