[WIP] API Separate file objects from readers #371
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From commit 2f893ffb89bf952df3cda4b3d9d11a06b5ac3070 of the
drewtils python project - release 0.2.0rc0. The main changes are
that the KeywordParser and PatternReader now operate on readable
buffers, not files to be opened. This means the
```
with KeywordParser(...) as parser:
<actions>
```
syntax is no longer supported. The motivation for this change
is in support of allowing parsers and files to also act on streams,
not strictly file names. This can allow the parsers to utilize
in-memory buffers like ``io.StringIO``, or, potentially, files that
have been compressed using the ``zipfile`` library.
Most of our readers work internally on the streams, because the
opened file is what we care about. But the changes may or may not
(likely the latter) be straightforward to implement. Thankfully this
branch / feature is all about breaking the file objects from the
readers so there already is some internal changes coming.
Living in serpentTools/base.py, this class provides a very basic interface that will begin the transition in separating file objects and file readers [GH CORE-GATECH-GROUP#335]. Concrete classes must provide a single class method, ``fromSerpent``, that should perform the following actions: 1. Accept a string or pathlib.Path object representing the file name, or a readable stream to be read 2. Create a new instance of the SerpentFile, e.g. DetectorFile, 3. Process the data in the file or stream, and 4. Return the SerpentFile instance with the new data A helper function is also provided that can handle this flexibility in input arguments - serpentTools.base.getStream. Used as a context manager, this function ensures a readable stream is returned. Future commits will begin implementing concrete classes piecewise.
This new concrete class implements the SerpentFile interface and a Mapping interface directly. The class method fromSerpent can be used to read a *_det<N>.m file and construct this new detector file representation. The parsing is performed a little differently now, more like a generator iterating over all Detectors found in the file using the new stream-based KeywordParser engine. The Mapping interface allows the DetectorFile to act like a dictionary mapping string detector names to Detector instances. Users can still use the setitem / getitem methods, but can only set Detector instances as values. Tests have been slightly modified to account for the new reading method, but the content of each detector is compared identically.
New results file object is provided that is responsible for storing data from a result file, while the newly modified ResultsReader is responsible for reading in data from a readable stream (e.g. opened file), and populating resdata, universes, and metadata dictionaries. This a partial implementation as the full comparison interface is yet to be added and tested. The unit tests have been updated only as they pertain to reading the files, with one exception. The result files have no need to store the "variables" that were used in the processing by passing variables and variableGroups to the class method. These are used by the reader, and accurately, as the updated tests indicate. However, I marked such tests as skipped for now until decisions are made on how to test this, if at all.
drewejohnson
added
Work in progress
discussion
API - Compatible
drewejohnson
commented
Jan 4, 2020
| __all__ = ["DetectorFile"] | ||
|
|
||
|
|
||
| class DetectorFile(SerpentFile, Mapping): |
There was a problem hiding this comment.
Might just want to make this inherit from dict rather than mapping. The __setitem__, update, and setdefault actions should be overloaded to ensure that DetectorFile is always full of Detector objects
|
Going to close this in favor of the approach described in #400 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wanted to put this up here to provide a space for discussion, but this is my plan for resolving #335 - the separation of classes for storing data and for reading data. The basic plan is this, for each file type we support, there will be a corresponding
Fileobject that inherits from the newserpentTools.base.SerpentFile. ThisFile, e.g. aDetectorFile, is responsible for simply storing data from the file, while theserpentTools.parsers.DetectorReaderknows how the plain text file is structured and can parse accordingly.The
SerpentFileis an abstract base class that provides the interface that concrete classes, likeDetectorFileare expected to implement. Currently this is a single class method that is responsible for reading a plain text Serpent file:The
fileOrStreamshould be file name to be read, or additionally a readable stream (like an opened file) to be read. This opens us up to work on more interesting representations of the output files, including in-memory buffers or, as I found this weekend, files inside a compressed zip archive. The additional key word arguments will interface with the settings module, and provide a consistent way to remove it - #339The goal for this PR is to have
serpentTools.readfetch the correctSerpentFilesubclass, and call thisfromSerpentclass method with optional filtering arguments. Most of these are pretty straightforward, as the parsing methods all work on opened files anyway. Those who useserpentTools.read(filepath)should experience no breaks in compatibility, but those who use the readers directly will.I'm almost positive all tests will fail at this point, mainly due to changes in the parsing engines
TODO