-
Notifications
You must be signed in to change notification settings - Fork 667
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
Expose public facing Cython in libmda layer #3734
Comments
|
Thanks @orbeckst! I will open The things that could be exposed as you say have to be sensible and stable.
This is just a rough first idea and is obviously will be subject to a bit of evolution as we go. |
@MDAnalysis/coredevs with this in mind would people be open to exposing |
Yes, why not? Do you envisage packaging them separately? |
So something that came up in a conversation somewhere else - |
We can move it within lib for sure. The aim was just one entry point for the cython if you want it so it doesn't really matter where it goes |
Is there a potential issue with cyclic imports? |
Possibly I would have to test. It’s only for .pxd cimports which act more
like C headers so I’m not sure if cyclical will be a problem.
The upside is that you can do stuff like
‘’’ from MDA.lib.libmdanalysis cimport timestep’’’
But the possibly confusing part is that the headers are elsewhere.
On Sat, 5 Nov 2022 at 9:08 am, Irfan Alibay ***@***.***> wrote:
We can move it within lib for sure. The aim was just one entry point for
the cython if you want it so it doesn't really matter where it goes
Is there a potential issue with cyclic imports?
—
Reply to this email directly, view it on GitHub
<#3734 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AF3RHCZDOJDIJDAGX35CDXTWGWCNDANCNFSM5ZZVE3MA>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
--
Hugo MacDermott-Opeskin
|
Are people happy with the name |
I’m fine with the name, just spelled correctly ;-). |
I'm happy with lib.libmdanalysis if that makes sense to folks / doesn't cause weird import issues. I take view that folks that will use libmdanalysis will understand the codebase more than the newcomers who might get confused by the presence of two lib* directories. |
I think we can close this now with the note that folks add their public facing cython to |
Is your feature request related to a problem?
As part of #3683, the
libmda
folder with its own__init__.pxd
was introduced to provide a single place to expose public Cython APIs for interoperability. For example if external Cython code wishes to create a Timestep directly one can use:This is similar to the strategy used in
Cython
itself whereby all of thelibc
andlibcpp
imports live in one place:For example
Describe the solution you'd like
As more modules are Cythonised as part of CZIEOSS4 Performance track changes, we should continue to add them to the public Cython API.
This however does require some decisions as to what is public from the Cython side and what should remain internal.
Alternately if people do not like this idea we should probably discuss it and now is likely a good time. :)
Describe alternatives you've considered
Have everything be private and internal to MDAnalysis.
The text was updated successfully, but these errors were encountered: