Feature/netcdf

[bruvio]

@bobturneruk @RyanJField

I drafted the PR as you suggested.

I wonder if you can suggest how to move with this?

at the moment I am reading the R implementation and, for now, translate read_array/write_array to python.

I would like to hear your view on this as I do not want to reinvent the wheel. For example, as I am not familiar with R, is there already a Python implementation for the R functions resolve_read/resolve_write?

[RyanJField]

The code for resolve_read and write are in the link functions if you read the R link_read / write functions you can see the code was put into resolve read / write to avoid duplication with the read / write_ functions.

[bobturneruk]

Pasting these in here in case they're useful:

• https://github.com/FAIRDataPipeline/rDataPipeline/blob/main/tests/testthat/test_read_array.R
• https://github.com/FAIRDataPipeline/rDataPipeline/blob/main/tests/testthat/test_write_array.R

I guess they specify what read_array and write_array should do (in a HDF5 setup). I guess the strategy here is to sidestep HDF5 for python and go straight to netCDF?

[bruvio]

@RyanJField @bobturneruk at the moment (after refactorin the code to expose resolve_read and resolve_write as in R)
I get an error when executing

[bruvio]

@bobturneruk @RyanJField

as far as this stands I am getting an error when executing fair run command in this branch.

as I ssaid I just exposed (i.e. refactored) two functions resolve_read and resolve_write to be similar to the R implementation and then serve for future implementation of read/write netcdf.

I would like a new set of eyes to have a look into this error as it is not related to the cli,

$ fair run simpleModel/ext/SEIRSconfig.yaml 
Updating registry from simpleModel/ext/SEIRSconfig.yaml
Traceback (most recent call last):
  File "simpleModel/ext/SEIRSModelRun.py", line 39, in <module>
    simpleModel.SEIRS_Plot(sm, model_plot)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/simpleModel/common/SEIRS_Plot.py", line 28, in SEIRS_Plot
    plt.savefig(save_location)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/pyplot.py", line 958, in savefig
    res = fig.savefig(*args, **kwargs)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/figure.py", line 3019, in savefig
    self.canvas.print_figure(fname, **kwargs)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/backend_bases.py", line 2259, in print_figure
    canvas = self._get_output_canvas(backend, format)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/backend_bases.py", line 2188, in _get_output_canvas
    raise ValueError(
ValueError: Format 'netcdf' is not supported (supported formats: eps, jpeg, jpg, pdf, pgf, png, ps, raw, rgba, svg, svgz, tif, tiff)

[RyanJField]

@bobturneruk @RyanJField

as far as this stands I am getting an error when executing fair run command in this branch.

as I ssaid I just exposed (i.e. refactored) two functions resolve_read and resolve_write to be similar to the R implementation and then serve for future implementation of read/write netcdf.

I would like a new set of eyes to have a look into this error as it is not related to the cli,

$ fair run simpleModel/ext/SEIRSconfig.yaml 
Updating registry from simpleModel/ext/SEIRSconfig.yaml
Traceback (most recent call last):
  File "simpleModel/ext/SEIRSModelRun.py", line 39, in <module>
    simpleModel.SEIRS_Plot(sm, model_plot)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/simpleModel/common/SEIRS_Plot.py", line 28, in SEIRS_Plot
    plt.savefig(save_location)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/pyplot.py", line 958, in savefig
    res = fig.savefig(*args, **kwargs)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/figure.py", line 3019, in savefig
    self.canvas.print_figure(fname, **kwargs)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/backend_bases.py", line 2259, in print_figure
    canvas = self._get_output_canvas(backend, format)
  File "/home/bruvio/Dropbox/work/UKAEA/pyDataPipeline/.venv/lib/python3.8/site-packages/matplotlib/backend_bases.py", line 2188, in _get_output_canvas
    raise ValueError(
ValueError: Format 'netcdf' is not supported (supported formats: eps, jpeg, jpg, pdf, pgf, png, ps, raw, rgba, svg, svgz, tif, tiff)

You need to use the file_type from the write block:

pyDataPipeline/data_pipeline_api/link.py

Line 26 in 6da168b

file_type = write["file_type"]

[bruvio]

with commit 16b7a14 I started to implement writing data to netcdf as the example set out by Derek.
@bobturneruk @RyanJField fell free to have a look and comment.

[bobturneruk]

Looks to be shaping up well. I'll have a go at getting it running on my machine and comment more.

[bruvio]

up to now there are a couple of wrapper functions to write 1d data as f(x) ,2d as f(x,y) and 3d data as f(x,y,z). Also there are wrapper functions to create groups and nested groups. Group creation is idempotent so creating a group that is already populated does not raise errors or deletes data. I still have to write wrappers to read data from netcdf. And then if you agree we can move to write read_array and write_array function as in the R implementation.

[bruvio]

@RyanJField @richardreeve @B0SKAMP @bobturneruk
commit c979fbf contains a first implementation of write_array.
there are also tests (no edge cases for now)

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
17 Code Smells

No Coverage information
18.3% Duplication

[bruvio]

@B0SKAMP @richardreeve @RyanJField @bobturneruk
Hi!
I refactored the code.
Hope meets the requirements.

let me know.

[bobturneruk]

I'm not sufficiently sure of the current requirements to say if this meeting them. This will need attention from @richardreeve, I think. I can look at the code.

[richardreeve]

Great, thanks @bruvio - I'll have some time to look at this tomorrow.

[richardreeve]

Hi @bruvio - I (actually met up with!) and talked through this with @B0SKAMP today, and since he has a much clearer understanding of the details than me, he said he'd talk to you about it early next week. It sounds like between the two of you you're converging on a really nice solution.

[bruvio]

with last bunch of commits refactored more the code to prepare headers better, now also the data is set into the headers when calling prepare_headers. I also added a bit more refactoring here and there.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
18 Code Smells

No Coverage information
26.3% Duplication

[bruvio]

I think we can start reviewing this.

this is a copy paste of an email exchange:

Currently all data is written in one go. Another interface can be provided to write in slices.

What we need to agree on is the internal format of the netcdf file.

The naming format is "grp1/grp2/..../grp/varname

The root of the netcdf file should have a schema version attribute.

I create a netcdf dimension with the "_dim" suffix when creating a dimension. Then a netcdf variable with the dimension name which references the "_dim" suffix.

When an array is created I give the dimension names. From these a retrieve the netcdf "_dim" dimensions. A netcdf variable is created refering to the "_dim" dimensions

Finally as discussed there is a possibility to pass custom attributes.

@RyanJField @richardreeve @B0SKAMP

Derek is not able to see this, he might be able to comment as well?
Thanks