Change HDF5FormatMetadata to not sort keys in the metadata

[peendebak]

The sorting of keys fails for instruments with an on_off value mapping (these are dicts with both str keys and non-str keys which cannot be compared).

With the default python dict being orderered, the sorting is not needed any more.

@astafan8

[codecov]

Codecov Report

Merging #1581 into master will increase coverage by <.01%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1581      +/-   ##
==========================================
+ Coverage   72.33%   72.33%   +<.01%     
==========================================
  Files         116      116              
  Lines       12388    12389       +1     
==========================================
+ Hits         8961     8962       +1     
  Misses       3427     3427

[astafan8]

Could you please add a test that would capture the "new" default behavior and the old behavior that is accessible with sort_keys=True?

[peendebak]

@astafan8 I updated the code style. Writing a test quite some work: the qcodes.DataSet is deprecated and currently there are no tests for the HDF5FormatMetadata, so it would mean writing tests for the entire class.

[peendebak]

@astafan8 There is still a warning from codacy. But fixing that might create a cascade of updates in the other formatter. Any suggestions?

[astafan8]

.. currently there are no tests for the HDF5FormatMetadata ...

I managed to find this file but there, only the "reading" is somewhat tested.

the qcodes.DataSet is deprecated

that's true, but there are still users of it, and we haven't done anything yet to explicitly show that "old dataset" is deprecated. Either way, this is NOT an argument against testing :)

---

So, i would still insist on writing a test, but just only for this class and this feature - one for .write_metadata(data_set) call (sort_keys=False is assumed and the sorting of keys is asserted), and another for .write_metadata(data_set, sort_keys=True) call (again, asserting the expected sorting). The test code that i saw here is quite short and i'm sure it is not much time for a user of the old dataset to implement these two simple tests. and actually, when you added sort_keys argument, how did you test that it worked? ;) you might still have that code, so just restructure it into a test, and done :) other users of the old dataset and this feature will be very happy to know that it actually works.

[astafan8]

as for the fixing of the codacy warning, i'd suggest to try to add **kwargs to the signatures of this method from the super classes. Hopefully the issue will be resolved.

[peendebak]

@astafan8 There is a warning from codacy about the parameters of the write_metadata method. I can update the write_metadata in the base class, but the argument sort_keys would not do anything for the base class. Is that the right solution?

[astafan8]

@peendebak About the warning from codacy. i'd suggest to update the write_metadata in the base class to include **kwargs, not specifically the sort_keys argument. Could you try that and see if it helps?

[peendebak]

@astafan8 Adding kwargs doesn't seem to satisfy codacy.

[astafan8]

please, also fix the signature in qcodes\data\hdf5_format_hickle.py:21 file.

yes, i see that Codacy still complains. What would help is to NOT have the sort_keys argument explicitly, but extract it from the **kwargs dictionary with smth along the lines of sort_keys = kwargs.get('sort_keys', True) (so that True is the default value). I prefer it less, but it's also fine. But, please, document that kwarg in the docstring (especially because you refuse to write a test :) ).

[peendebak]

@astafan8 Codacy is happy now!

[astafan8]

great :) last thing - in the dcostring for the sort_key argument, please mention that the default value is False, and then I'll merge.