-
Notifications
You must be signed in to change notification settings - Fork 1
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
Make SANSND plugin ready for ncrystal v4 #1 #6
Make SANSND plugin ready for ncrystal v4 #1 #6
Conversation
So the CI is again not running, most likely since there are not yet any workflows in the main branch. |
Hi Thomas, thank you for the little intro to the new plugin system. Great news that we can just pip install the plugins, sounds very handy. I went through the commits and it seems all good. Didn't understand a lot about the infrastructure's updates, but probably it's not something that should concern me. I tested the plugin ncplugin-fix-SANSND-for-ncrystal4 with the ncrystal 4 release candidates and it looks okay. It would be definitely interesting to hear more about this new infrastructure. For example, I would be interested in providing access to the 'ncplugin-SANSND_nanodiamond.ncmat' upon installation, without copying the file or ncdatasrc.addCustomSearchDirectory() P.S. sorry for the garbage in the testcode folder. I promise I will clean it. |
Hi @nicriz Thanks for the quick merge and verification that it all works! Feel free to reach out if you want a video call with a more in-depth intro in a few weeks time. In the mean-time, note that after installing your plugin, you can see with It will be even easier to add a static plugin which only has data files, in that case you can do it without any CMake. I am working on an example here: https://github.com/mctools/ncrystal/tree/tk_reorder_pyproject/examples/plugin_dataonly One thing to be aware off, is that you now have a file NCTestPlugin.cc, in which you can add some test functions that verifies the plugin works. I will set up a CI workflow in the ncrystal repo which monitors all the "well known" plugins in other repos, to see that they build and load, and also that their test function runs file. That way I can better avoid accidentally breaking a plugin when I create a new ncrystal release. |
Hi again, Yes, you're right, the key was to use the full name "plugins::SANSND/ncplugin-SANSND_nanodiamond.ncmat" in the cfg string. Amazing! I might add one file for each model then. I saw the new Test functions infrastructure and I will think about a test for the plugin to insert in the CI workflow. Let's meet in two weeks or so. |
FYI @nicriz , I just released ncrystal-pypluginmgr 0.0.4, so with that you can simply install your plugin and then afterwards run:
To verify that the plugin exists, and run it's test function. |
Attention in particular @nicriz (and possibly also @XuShuqi77 @marquezj @dddijulio).
(FYI most text here is similar to highness-eu#1 except for the last paragraph)
These changes are needed to make the plugin ready for NCrystal 4, which will be out in a few weeks.
It can already be used in the meantime however, by pip installing the
ncrystal-core
andncrystal-python
packages which contains release candidates of the new code (but do NOT install the package named justncrystal
until NCrystal4 is out). For refererence,ncrystal-core
contains all binaries, headers, cmake, andncrystal-config
-- andncrystal-python
contains theNCrystal
python module and all the other command line tools (includingnctool
).To install the plugin, you can now simply point pip directly at the repo:
And then it will work with no further need to tweak environment variables or whatnot.
Of course, until you accept this PR, the above command won't work. But you can try out the code in this PR with:
This PR also adds a unit tests, that ensures a basic functionality test of the plugin on linux/mac/windows.
@nicriz let me know if at some point you need a bit more introduction to all this. For now I migrated the main plugin code, added a little test in NCTestPlugin.cc, and completely left all the mess inside
testcode/
alone :-)