search

NCAS-CMS / cfdm

A Python reference implementation of the CF data model
http://ncas-cms.github.io/cfdm
MIT License
28 stars 11 forks source link

Detection of duplicates in API reference, some to remove #277

Closed sadielbartholomew closed 10 months ago

sadielbartholomew commented 10 months ago

Took a slight tangent from my reviewing of #272 when considering API reference coverage and spotting there are some methods listed multiple times in the same file or even under the same sub-heading.

I thought I'd make a small update to the check_docs_api_coverage script so we can automate the detection of those when we are already checking that they are all present. So with this change we are checking they are all present precisely once and not just not zero times like before (sub-string method name complications are dealt with - see code comments), unless we've deliberately chosen to duplicate them under different headings. A warning is raised reporting any duplicates for consideration.

I've removed any duplication that seems unintended in the API reference source restructured text, thought left in some cases where it looks like we might want to put methods under multiple headings. Namely, at PR opening the script still reports these which I haven't touched:

$ ./check_docs_api_coverage
WARNING: some methods are listed multiple times inside one class file/page. Decide if the duplicates are intended and if not remove them. They are:
cfdm.Data.del_calendar
cfdm.Data.get_calendar
cfdm.Data.has_calendar
cfdm.Data.set_calendar
cfdm.Data.sum
cfdm.Data.unique
cfdm.Field.del_data_axes
cfdm.Field.get_data_axes
cfdm.Field.has_data_axes
cfdm.Field.set_data_axes
cfdm.core.Coordinate.del_bounds
cfdm.core.Coordinate.del_data
cfdm.core.Coordinate.set_bounds
cfdm.core.Coordinate.set_data
cfdm.core.Data.del_calendar
cfdm.core.Data.get_calendar
cfdm.core.Data.has_calendar
cfdm.core.Data.set_calendar
cfdm.core.Field.del_data_axes
cfdm.core.Field.get_data_axes
cfdm.core.Field.has_data_axes
cfdm.core.Field.set_data_axes
cfdm.core.PropertiesDataBounds.del_bounds
cfdm.core.PropertiesDataBounds.del_data
cfdm.core.PropertiesDataBounds.set_bounds
cfdm.core.PropertiesDataBounds.set_data

All non-private methods are documented

as opposed to, previously, a longer list:

WARNING: there are methods which are listed multiple times in one class file/page. Decide if the duplicates are useful. They are:
cfdm.Data.del_calendar
cfdm.Data.get_calendar
cfdm.Data.has_calendar
cfdm.Data.set_calendar
cfdm.Data.sum
cfdm.Data.unique
cfdm.Domain.apply_masking
cfdm.Domain.climatological_time_axes
cfdm.Field.climatological_time_axes
cfdm.Field.del_data_axes
cfdm.Field.get_data_axes
cfdm.Field.has_data_axes
cfdm.Field.set_data_axes
cfdm.NetCDFArray.get_address
cfdm.core.Coordinate.del_bounds
cfdm.core.Coordinate.del_data
cfdm.core.Coordinate.set_bounds
cfdm.core.Coordinate.set_data
cfdm.core.Data.del_calendar
cfdm.core.Data.get_calendar
cfdm.core.Data.has_calendar
cfdm.core.Data.set_calendar
cfdm.core.DomainAxis.construct_type
cfdm.core.Field.del_data_axes
cfdm.core.Field.get_data_axes
cfdm.core.Field.has_data_axes
cfdm.core.Field.set_data_axes
cfdm.core.PropertiesDataBounds.del_bounds
cfdm.core.PropertiesDataBounds.del_data
cfdm.core.PropertiesDataBounds.set_bounds
cfdm.core.PropertiesDataBounds.set_data

All non-private methods are documented

Happy to remove those duplicates from the first snippet too, though? But the main thing is the automation of the check and that I've removed methods duplicated under the same heading, which can be blitzed to tidy.

davidhassell commented 10 months ago

Hi Sadie - thanks for this. I reckon that we could keep cfdm.Data.sum and cfdm.Data.unique and get rid of the rest. What do you think?

sadielbartholomew commented 10 months ago

I reckon that we could keep cfdm.Data.sum and cfdm.Data.unique and get rid of the rest. What do you think?

Sounds good to me. I'll update the PR shortly to get the duplicates other than those removed, in the least appropriate location.

sadielbartholomew commented 10 months ago

@davidhassell I have now updated the PR to remove the majority of the rest of the duplicates, so that the output of the script is:

$ ./check_docs_api_coverage 
WARNING: some methods are listed multiple times inside one class file/page. Decide if the duplicates are intended and if not remove them. They are:
cfdm.Data.sum
cfdm.Data.unique

All non-private methods are documented

in line with your idea to leave just those two under cfdm.Data.

So this should be ready for re-review. Thanks.