search

Reading-eScience-Centre / edal-java

Environmental Data Abstraction Layer libraries
Other
39 stars 30 forks source link

Fixed CdmUtils to use GridDataset directly #39

Closed lesserwhirls closed 8 years ago

lesserwhirls commented 8 years ago

CdmUtils was using a factory to create the GridDataset. However, the factory now returns a Coverage (new) rather than a GridDataset. In an attempt to get this working using netcdf-java 5.0, this PR changes CdmUtil to use GridDataset directly.

Also added .iml files to gitignore (.iml come from intellij IDE)

lesserwhirls commented 8 years ago

Note that once netcdf-java 5 becomes stable, the use of GridDataset should be updated to Coverage, as Coverage is more flexible.

guygriffiths commented 8 years ago

What is the behaviour of new GridDataset(ncDataset) when there are no grid datasets present in the file?

Also, the code is currently using netcdf-java 5.0.0, and the factory returns a FeatureDataset. Which version are you using?

lesserwhirls commented 8 years ago

What is the behaviour of new GridDataset(ncDataset) when there are no grid datasets present in the file?

I think that gridDs.getGrids() will return an empty list if no grids are found.

Also, the code is currently using netcdf-java 5.0.0, and the factory returns a FeatureDataset. Which version are you using?

I am the code from my branch:

/lesserwhirls/thredds/tree/ncwms2

which is a branch off of the a very recent commit in the 5.0.0 brach of the thredds repo:

Unidata/thredds@91b59822f85186bc48076e855b65c177aedf9f71

I think the behavior of the factory has changed recently. I am going to try to get nighty snapshots of 5.0 on our nexus repository, if that will help.

guygriffiths commented 8 years ago

OK, I'm going to close this request because:

lesserwhirls commented 8 years ago

Sounds great! If I find anything else, is there a certain branch that you prefer that I would make a pull request against? I'd be happy to help migrate from GridDataset to Coverage once you all are ready and once netcdf-java 5.0 becomes beta.

Thanks again, and cheers!

On Wed, Nov 25, 2015 at 9:13 AM, Guy Griffiths notifications@github.com wrote:

OK, I'm going to close this request because:

  • I've implemented the exact same behaviour but I first check if 0 grids are found and if so, throw an exception. I've just committed to the develop branch and pushed it
  • Nothing gets merged into the master branch except during a release

— Reply to this email directly or view it on GitHub https://github.com/Reading-eScience-Centre/edal-java/pull/39#issuecomment-159658666 .

guygriffiths commented 8 years ago

That would be very helpful, thanks! In general all pull requests should be done against develop. We follow a gitflow-esque pattern which can be summarised as:

That way, master always holds the state of the latest release

lesserwhirls commented 8 years ago

Makes sense to me. Also, we've started pushing nightly maven artifacts (snapshots) to our nexus server for the 5.0 branch of THREDDS if that is of use.

On Wed, Nov 25, 2015 at 9:45 AM, Guy Griffiths notifications@github.com wrote:

That would be very helpful, thanks! In general all pull requests should be done against develop. We follow a gitflow-esque pattern which can be summarised as:

  • All actual work goes into the develop branch
  • When ready, we create a release branch off develop, update the pom files etc.
  • Merge release branch into master

That way, master always holds the state of the latest release

— Reply to this email directly or view it on GitHub https://github.com/Reading-eScience-Centre/edal-java/pull/39#issuecomment-159667938 .