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 'core' prefix to entry points defined by aiida-core #4914

Closed
ltalirz opened this issue Sep 19, 2019 · 3 comments · Fixed by #5073
Closed

add 'core' prefix to entry points defined by aiida-core #4914

ltalirz opened this issue Sep 19, 2019 · 3 comments · Fixed by #5073
Assignees
Milestone

Comments

@ltalirz
Copy link
Member

ltalirz commented Sep 19, 2019

In order to be in line with rules for plugins and to avoid polluting the global namespace, the entry points defined by aiida-core should be prefixed by 'core.', e.g. core.structure.

See also discussion here

Planned for v2.0.0

@sphuber
Copy link
Contributor

sphuber commented Nov 6, 2019

Just happened to come across this and wanted to mention that core.structure might not be the best example as we might want to move that class to a separate plugin package. core.int, core.float etc. would of course be internal types that would need the prefix

@ltalirz
Copy link
Member Author

ltalirz commented May 5, 2021

@sphuber just checking what's the consensus on that one... do we leave it untouched in 2.0?

@sphuber
Copy link
Contributor

sphuber commented May 5, 2021

Hmm, forgot about this one. Originally I would have said to coordinate this with the moving out of material science specific plugins to a separate package since that would anyway require entry points to be changed. But maybe we can already preempt this change for the plugins that should stay regardless. I am thinking if we can do this in a backwards-compatible way and deprecate the old entry point names.

We could change the entry point names for internal plugins (Int, Float) etc. to add the core. prefix and add a database migration that updates the node_type from affected nodes, e.g. data.int.Int. would become data.core.init.Int.. This way, after the migration anyone loading a node with data.core.int.Int. would get the correct class back. The only thing that would break in this case (if I am not forgetting anything) is the line

Int = DataFactory('int')

which would have to be changed to

Int = DataFactory('core.int')

However, I think we can deprecate this by having a look up list in the DataFactory that knows about the migrated entry points and in those cases, prints a warning notifying the user of the new entry point name. Then at some point we can remove this and everything should be fine. I think this approach might work and therefore may be interesting to consider adding for v2.0

@ltalirz ltalirz transferred this issue from aiidateam/aiida-registry May 5, 2021
@ltalirz ltalirz added this to the v2.0.0 milestone May 5, 2021
@ramirezfranciscof ramirezfranciscof assigned sphuber and unassigned ltalirz Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants