Protected variables and general code refactor

[MattiasFredriksson]

!!!Note this is rebased onto the poetry branch to avoid conflicts (once poetry is merged the diff should make more sense I hope).

Protected variables and general code refactor

The goal of these changes was/is to prevent users from accessing Header/Manager/Group/Parameter variables directly and instead interacting with them through the class interface. This should avoid code duplication and/or erroneous behavior (e.g. mixing different ._datatype instances). Tests are also included to verify that the extended class interfaces behave as expected. The update contains the following:

Major:

• Added single underscore to 'protected' variables to prevent users from accessing them directly.
• Added accessors through @properties or group_values(), param_values(), group_items(), ...
• Extended the classes with functionality for adding/renaming/removing Group/Parameters.
• Updated the package to use the new functions internally.

Minor:

• Some convenience functions such as Parameter.int_array or .float_array, returning an array based on .bytes_per_element
• Some renaming of protected functions and docstring rewording.
• Might have missed something.

Downsides? it might break existing code accessing objects from the internal dictionaries. However, I hope the changes are not disagreeable as I believe refactoring the access to the internal variables is relevant. To let the code remain similar e.g. a previous call to say reader.groups.values() would now be reader.group_values() or reader.group_items() for .items().

Hopefully the changes are agreeable, and hopefully I didn't missed to mention some change(s).

BR Mattias Fredriksson

[AKuederle]

Thanks for separating the refactoring out of the major merge request! Looks very reasonable to me. The code also looks good to me, but I don't feel like I fully understand yet all the intended functionality, as I am not that deep into the codebase yet. However, I think that is fine, as long as we don't break the major usecases (which I don't think we do).

We should probably add a CHANGELOG.md to the project to document breaking changes going forward, but I will handle that.

I merged the poetry branch (thanks for directly working of of that :) ). This required another rebase. Could you rebase this branch as well on the current master? Then we should be good to go!

Thanks again!

[MattiasFredriksson]

Updated the branch by rebasing it on top of the current master. Accidently merged it when I pulled down the updated master and had to reset it to correct the error. Will see if I can do another pull request for further changes for next week.

[MattiasFredriksson]

To expand on some further reasons to why it's not a great idea to manually edit to underlying dictionaries storing the Group/Param entries is that they are associated with both a string name (the group/param name) and an 'index' integer (there is no guarantee that groups or parameters present in a file forms a continuous index sequence!). Currently a Group/Param dictionary contains duplicate references, one keyed by name and the other by its index (and this has always been the case). To make this behavior consistent incase someone wants to add/change data in a file, providing a consistent interface to do so will probably prevent multiple headaches.

[AKuederle]

Awesome! And thanks for the explanation!