Compress

[HoWol76]

Allows output in compressed form, resolves issue #122

New namelist property: compressed. Defaults to False, if True, arrays are compressed on output.

Changes method _var_strings by two methods:

_compress converts the v_values list to a compressed list where each element is a 2-element list with the number of repetitions and the original element.

_f90comprepr converts each of these individual 2-element lists into the correct compressed format.

Several tests were added for this feature.

[HoWol76]

TODO: No changes have yet been made to the version number or the documentation.

[coveralls]

Coverage decreased (-0.2%) to 99.442% when pulling f47e71d on HoWol76:compress into 7de6c44 on marshallward:master.

[HoWol76]

The coverage decrease is not due to my code changes. All my code is covered by unit tests.

[marshallward]

Thanks Holger, this looks like very nice work, and thank you also for adding the tests. I am very happy to merge this.

The code is good, the only real issue I have is with describing it as compression. The Fortran standard often describes as a "repeat" or "repeat factor". For example, the "data-stmt-repeat" is the label used for the repeat counter in a data statement. (Annoyingly, the IO section calls them "r* form" which really doesn't help with naming conventions. My old Metcalf/Reid/Cohen book also calls them "repeat counters". (That name is suspiciously absent from the standard, but nevermind that for now...)

The other reason is that "compression" can also refer to more elaborate forms of data compression. I guess this is a kind of compression, but might not be what comes to mind for most people.

I can make changes here if you are happy with them, or you are welcome to do them yourself.

In the future, we could modify this so that repeat_counter also takes an integer argument, which could represent a minimum count to enable repeated values. But let's leave that for another time.

---

No worries about the coverage. I think something changed with respect to YAML support on either the Travis or Ubuntu side of things.

[marshallward]

Also _compress could probably be replaced with something like this:

list([len(list(x)),i] for i, x in itertools.groupby(values))

[marshallward]

I have one more comment which is more of a long-term concern and which need not be addressed right now.

I am a little concerned that we are now implicitly purposing the list-of-list [[r],[c]] as a temporary representation of a repeated counter, while also using also use list-of-lists in other plafces to represent multidimensional arrays. Because we process multidimensional list-of-lists before reaching this point, and because we use the flag to catch the _f90repr call and instead pass to _f90comprepr, I think we avoid the issue of duplicate representation.

I have not been able to find any case which causes problems, and it obviously passes all the current tests, so I don't mind merging this. But we may want to at least add a comment describing the potential conflict in any future work.

[HoWol76]

I've taken your suggestions to heart.

1. It's now called repeat, not compress. This might still be counterintuitive, as I would call 1, 1, 1 repeating, and 3*1 non-repeating.
2. I've created a new tiny class RepeatValue that simply stores the value and the times that value is repeated. This also means that we can move this back into the _f90repr method and distinguish by the type.
3. I'm using itertools as you suggested.
4. I just noticed that I didn't run the codestyle checker. Let me to this right now.

[HoWol76]

Still TODO:

• Fix version number
• Update documentation
• when reading namelist, set repeat property correctly.

[marshallward]

Thanks, I like this very much. I think there could also be additional benefit by introducing more of these micro-classes.

I agree that repeat is still a bit counterintuitive. What do you think of repeat_counter? It is heavier than repeat, but I think it says what it means. (Like the other logical flags, I suppose the use_ is implicit...)

---

Don't worry about versioning or version numbers, I will handle that after this is merged.

[marshallward]

I think I will go ahead and merge this, despite a few minor issues (repeat counters with nulls?). This should not effect default behaviour and will only be noticed when this feature is enabled. The other issues like documentation can be sorted out in future PRs. If no objections from you, I'd like to merge it.