-
Notifications
You must be signed in to change notification settings - Fork 5
add FDMNES output to webapp, some i/o tweaks #10
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
Conversation
…text (no explicit IO)
|
@newville it looks great to me! Thanks for implementing this so quickly. I did only a quick look, but I am tempted to merge it and release I have only observed that the Larixite version does not get exported correctly in the FDMNES input file. Furthermore, it would be great to follow good practices and add some tests for this, if possible |
|
I would like to try this on a production web app before merging. I think it should all be OK, but adding generared zipfiles probably needs testing on a real server. Hopefully today! |
|
OK, I agree. There is no hurry from my side. This can wait. In the meantime, I will go deeper in the code, especially for the part separating the calculation from I/O. I think I will propose few changes there. |
|
FWIW, the web app at https://millenia.cars.aps.anl.gov/larixite now redirects to https://larixite.seescience.org/ (which is the same server) and is using this branch. So, I think this is working well enough to release. |
|
@newville I have lost track of this pull request. Should we simply merge it? |
|
@maurov yes, and thanks for the reminder. The web app is working to generate zip files of FDMNES files, so it should be fine to merge this. |
This adds FDMNES outputs to the larixite web/Flask app (addressing #6).
It also adds a Zip file to download all outputs, including the CIF file.
This also adds a couple of convenience functions for
structandfdmnesto allow generating structure results not from a filename/Path, but from the text of a CIF file, and not writing files but returning the text of the output files. That is, without doing I/O in the calculation function (Separating calculation from I/O is sort of like the MVC discussions -- generally a good idea, sometimes hard to always follow in practice).Unfortunately, pymatgen's XYZ function needs a real file or Path, and cannot use a
StringIO(text)buffer. If that gets resolved, thenget_structurecould callget_structure_from_text, but at the moment that is sort of clunky, so I ended up not doing that, and leaving sort of duplicate code that should be cleaned up (like, whenpymatgen.io.xys.XYZcan read from StringIO).Anyway, this branch works for me for local generation of FDMNES files and Zip Files of all outputs.