search

aodn / python-aodntools

Repository for templates and code relating to generating standard NetCDF files for the Australia Ocean Data Network
GNU Lesser General Public License v3.0
10 stars 3 forks source link

Fixing the temp file permission bug, plus Velocity gridded product code #176

Closed mphemming closed 1 year ago

mphemming commented 1 year ago

Fixing the temp file permission bug

So the issue is that the temp file is still open in Spyder, meaning that shutils cannot move the file at the end of the main aggregator function.

I edited line 214 to include 'fd' as output: _, temp_outfile = tempfile.mkstemp(suffix='.nc', dir=output_dir) to fd, temp_outfile = tempfile.mkstemp(suffix='.nc', dir=output_dir).

I can then close the object adding the following code to line 355 prior to using 'shutil.move':

os.close(fd)

This now works without an error for both 'aggregated_timeseries.py' and 'velocity_aggregated_timeseries.py'.

Velocity gridded timeseries product code

An attempt to create a velocity gridded product using the groupby method, as used to produce the other gridded products. Gridded VCUR, UCUR and WCUR are provided, along with metadata.

The code could probably do with improving - I tried to run the loop in parallel with no luck. Although for one year's worth of velocity data it takes about 4 mins on my laptop without remote access to a server so not too bad.

Attributes are mostly correct, but a few attributes might need tweaking or new ones added. Also keep in mind that the method of adding attributes might not be satisfactory for the AODN. For example, I have copied the abstract, title and lineage attributes into the script rather than relying on the json template.

This product is probably good enough for NSW-IMOS purposes, but we may develop different versions, or improve the code in future. If so, I can create a new pull request.

mphemming commented 1 year ago

@mhidas Please do not accept the first commit, I had to delete the nc files in the forked python-aodntools/tree/master/test_aodntools in order to clone my fork to my local directory as it complained.

mhidas commented 1 year ago

Let me know if you are able to make those changes or if you need help. If it's easier I can just pull out your changes from the second commit into a new PR and you can review them?

mphemming commented 1 year ago

Let me know if you are able to make those changes or if you need help. If it's easier I can just pull out your changes from the second commit into a new PR and you can review them?

@mhidas I think I've fixed it? reverted the velocity gridded product code and deletion of NetCDF files. I've also changed the line where I include the new code for the aggregated products (just after tempfile.mkstemp() is used), and now also do the same for the velocity hourly product. Let me know if I need to do anything else.

I can create a new pull request for the velocity gridded product after this request if you are interested.

mhidas commented 1 year ago

Thanks @mphemming . That's better, but still not quite there. If you have a look at the list of changed files, you can see your velocity gridded code is still in there, and there are still 3 test files missing. It gets a bit messy when you start revering bits & pieces...

Might be easiest to start a new branch and just apply the changes you want. If you're feeling adventurous you could just cherry-pick the 3 commits here that you need. In fact I just did that and created a draft PR for you #178 -- if you like we can just review and merge that one.

mhidas commented 1 year ago

Closing this in favour of #178, with the velocity gridded code to come in a future PR