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 dwave.samplers and dwave.composite namespace #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arcondello
Copy link
Member

@arcondello arcondello commented Dec 24, 2018

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.

@arcondello arcondello self-assigned this Dec 24, 2018
@arcondello arcondello force-pushed the feature/dwave-samplers-namespace branch from 72b4a55 to 8149fa1 Compare December 27, 2018 17:28
arcondello pushed a commit to arcondello/dwave-ocean-sdk that referenced this pull request Aug 20, 2019
Copy link
Member

@randomir randomir left a 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__)))
Copy link
Member

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?

Copy link
Member Author

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'))
Copy link
Member

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?

Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member Author

@arcondello arcondello Nov 12, 2019

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.

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants