oerc0122 / castep_outputs

Parser for CASTEP output files
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Consider object-oriented Block #149

Closed ajjackson closed 1 month ago

ajjackson commented 1 month ago

I don't like to recommend creating new classes where a Python built-in will do fine...

but looking at this return section of get_block (and the corresponding out_fmt parameter)

https://github.com/oerc0122/castep_outputs/blob/0512ad1edeed551de96caff831f1f79d5e3020d2/castep_outputs/utilities/castep_res.py#L47-L55

there is a strong code smell that this function should be consistently returning a Block object which provides the features or conversion to these other classes.

Specifically:

oerc0122 commented 1 month ago

Very true, this would also tie in well with exploiting the FileWrapper features of line-nos and detailed error reporting from file. Could require a deep bit of restructuring though.

ajjackson commented 1 month ago

That could potentially be done quite non-destructively: just maintain the str and iter interfaces to main information while adding methods to access the deeper stuff.

ajjackson commented 1 month ago

Ah, on closer inspection I see that that StringIO is the default and so used quite a lot. But it is generally used as an iterator over lines, so the proposed interface would drop into the functions expecting TextIO and Mostly Just Work.

Currently if I read/write code operating on a block, I have to check back and see whether the block is a list or a StringIO to understand what indexing/methods are allowed.