Moves fms_diag_yaml_obj to fms_diag_yaml

[uramirez8707]

Description
Moves fms_diag_yaml_obj to fms_diag_yaml
Sets the variables of diag_files and diag_fields types to private
Uses DIAG_NULL to initialize optional values.

Fixes # (issue)

How Has This Been Tested?
make passes with intel and gnu

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules
• New check tests, if applicable, are included
• make distcheck passes

[thomas-robinson]

@ngs333 needs to take a look at the style to make sure we're all on the same page.

[thomas-robinson]

@ngs333 Do you want these to all say that they return a copy? Or is it redundant?

[ngs333]

@thomas-robinson
If all , or almost all getters, return copies, then I think just easiest to say in a one sentence (instead of in each getter documentation) paragraph in the intro class description something like:
"All getter functions (functions named get_x(), for memeber field named x) return copies of the member variables unless explicitly noted."

However, we cant have the situation where we say in one place the getter returns a copy of the memeber variable and in another place (e.g. the getter documentation) that it returns the member varible itself. So if the getter is to have documentation (right above the definition), then it should say "It returns a cpy of the ..."

[thomas-robinson]

OK, all of these functions return a copy, so I think it's safe to just put it in the documentation of the derived type. @uramirez8707 can you add that to the documentation?

[thomas-robinson]

these need to have a list of functions, not just a general USE. I probably did that...

[thomas-robinson]

Update the type documentation to reflect that all get functions/routines return a copy of the variable being requested.

[ngs333]

Is a string version of file_write really needed? If so, then every place file_write is set, string_file_write needs also be set.
Might it be safer or easier to just use a funcion like : get_file_write_str() if (file_write .eq. .true) return 'true' else return 'false' ?

[uramirez8707]

string_file_write is set by the parser and that is used to to set file_write.

FMS/diag_manager/fms_diag_yaml.F90

Lines 214 to 215 in 2c218d0

	call diag_get_value_from_key(diag_yaml_id, diag_file_id, "write_file", fileobj%string_file_write, is_optional=.true.)
	if (fileobj%string_file_write .eq. "false") fileobj%file_write = .false.

Yeah, saving string_file_write is not really necessary, I was planning on changing this in the #future.

[ngs333]

What does this really mean: "If this field is absent, then the will be equal to the frequency for creating new files". In particular, what does mean. Is it a different field than file_duration. Seems like somewhere (maybe not necessarily in this module) there should be another function to encasulate the functionality. Like maybe caculate_actual_file_duration( diagYmlFile_type.file_duration, ... ) ?

[uramirez8707]

If the file_duration/file_duration_units is present (e.g 6 days) , diag_manager will write a new file every 6th day from the start time with data at whatever the new_file_frequency is. (e.g hourly data or whatever)

If the file_duration/file_duration_units is not present, diag_manager will write a new file at whatever the new_file_frequency is.

This will probably change, but we are not there yet.

[ngs333]

I think that if a comments switches from a description about the current class to something that happens in another class, then the comments should make that switch obvious. Perhaps change the comment to "If this field
is absent, then the diag_manager will use a file duration that will be equal to the frequency for creating new files." ?

[ngs333]

Approving, but with comments and questions.