Closed Mateasek closed 4 years ago
I'm in favour of this, but I think @CnlPepper is the best person to ask about changes to the architecture.
I think this sounds sensible as well.
General comment, then a specific one.
There are two tidy ways of implementing file reads from multiple formats, either:
1) multiple class method on the class to initialises from a file handle/filename 2) separate import and export methods
When I encounter messy formats like those commonly encountered in fusion, I tend to pick the cleanest or devise a clean file format and add methods to handle that format directly on the class. This implies it is the "intended" method of sharing data. All other formats are them handled by import or export functions that sit outside the class. The Raysect mesh class is built like this.
I would make any reading routine return an instance of the class, not the data in an intermediate form. The point of the class is to represent that data object.
It seems in this case you have multiple versions of a file format and can automatically determine the correct reading routine. Build an importer for each version, the user can then call them explicitly if they find an issue with the auto detection. Then make an automatic detection read function separately that picks between them. You'll be the best judge if this should be a class method or not. Though my feeling is it probably shouldn't be as some of the methods it calls will lie outside the class.
Thanks for suggestions @CnlPepper. I will work in the changes you propose. I was not comfortable with passing back the data in some intermediate form, but this solution did not come to my mind.
closed with PR #19
I propose the following changes for the eirene.py and Eirene class.
File parsing will be moved outside of the Eirene class and the file structure will become the following: eirene/ -eirene.py -fort44/ -read.py -version1.py -version2.py
file read.py will contain a function eirene_fort44 and a dictionary with available file versions and assigned parsing functions. The eirene_fort44 function would open the file, identify the version and call corresponding parsing function from version.py. The data would be returned in the format required by Eirene class. This way a single parsing function can be serving multiple file versions if they are the same and also this structure allows an easy expansion of the parsing function list, if there is need.
The Eirene class will become only a data container holding simulation results. A classmethod from_fort44 would call the eirene_fort44 to initialize Eirene class. Later, the class can obtain some more data handling and exporting methods and etc., if wanted.
Please let me know if you agree or if there are any suggestions and I will start working on the changes.