-
Notifications
You must be signed in to change notification settings - Fork 17
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
Public API #790
Comments
A first effort should be made in 0.2, to declare which is the interface we're trying to avoid breaking. |
Let's try to keep a list of what is going to be included in and excluded from the interface in 0.2. In
Out
|
I agree with the summary and in/out.
Shouldn't this be Regarding the "doubts":
Will depend on #970. I am not sure if in the end you will give a try to remove
So far it seems to me that they are not needed for platform creation. For other usage, maybe we just need to pay the price of exposing only the things we need through
These may be needed for advanced users who want to use
Personally I found this useless even in 0.1 and my opinion remains the same. However, I would guess that the reasons we implemented it also remain the same, so most likely we need it.
We certainly need it, because there is no other to specify even a number of shots (different than default) when executing something. That being said, I am not a huge fan of it, so if you have suggestions to simplify they are welcome.
I think it is needed if someone wants to use the pulse API directly, not through the native gates as we typically do. If |
Oh, yes, definitely.
I will, let's see.
Agreed.
Ok, let's keep it temporarily "out" then, and provide this as a further feature at a later stage.
The problem is that now we're committing to stability, so we either remove it now or next is in 0.3 (and we have to maintain until then).
Yeah, that's the thing I was unsure about. An option is to keep a
Ok, but this to me sounds a good argument to expose them. |
For the time being, I will apply a simplified strategy for optional dependencies, i.e. concerning drivers. However, we may want to reassess the issue later on, as it is not that trivial, and I'm not sure which is the best way to handle them since I'd like:
Pandas as a whole function (and module, https://github.com/pandas-dev/pandas/blob/main/pandas/compat/_optional.py) dedicated to the purpose, and even the But since the problem is non-trivial, I would adopt a more poor-man solution, and then reconsider. |
Since Qibolab is now being used, and we transitioned to proper releases, in order to make the semver conversion even more meaningful we should define what is part of the public API of Qibolab.
The convention in Python is that everything that is accessible passing through a chain of non-underscore-led identifiers is public, i.e.:
However, if you use
ciao._come.va
insideciao
, importing directly the identifier, i.e.this becomes accessible as
ciao.va
, and thus public again.In general, the idea is to keep all the definitions internal, and exports only the public API through suitable
__init__.py
and modules imported by that, as a restricted hierarchy within the package, just made for re-exports.Often, a
_core
subpackage, or something like that, is used to separate what is internal from the public API.To simplify the management of the public API, there are different tools available in Python's module system:
__all__
: is an attribute that specifies what is imported from this module when doingfrom mymodule import *
- usually I would discourageimport *
, but it's convenient for re-exports__getattr__
is customizable at module-level, and it could be used to provide dynamic loading (that for us could be useful to export the drivers)__dir__
is customizable in the same wayNumPy is quite a full-fledged example of how all of these are used to define the public API. It is also over-complicate for us, since they have decades of legacy code and users to support, but it's interesting to navigate their
__init__.py
structure.This will help to identify which changes are actually breaking, and which are not.
(In principle, whatever is not part of the public API should also be kept internal, and not documented in Sphinx - I guess this should work automatically, ensuring meaningful
__dir__
values - and it should not be an incentive to avoid docstrings, even for internal elements)The text was updated successfully, but these errors were encountered: