Clean duplicates off sites.json

[pllim]

From @eerovaher (https://github.com/astropy/astropy/pull/12721/files#r786773846)

Raising warnings about duplicated names sounds like a good idea, but it should not be implemented until the sites.json file in astropy-data is cleaned from duplicates, otherwise the users would receive many warnings that they can't really do anything about. Removing the duplicates from sites.json would reduce the number of usable keys for anyone not using the bleeding edge version of astropy, so it doesn't seem it would be a good idea to do that quite yet.

When it comes to checking for duplicated names for different sites then it wouldn't be too difficult to add a test to astropy-data that would check for that. The output of astropy.coordinates.EarthLocation.get_site_names() shows that currently there aren't any duplicated labels across sites other than the empty string that this pull request takes care of.

[eerovaher]

What @pllim has quoted might not be clear without context, so I'll write an explanation here. https://github.com/astropy/astropy-data/blob/gh-pages/coordinates/sites.json contains data for the astropy.coordinates.EarthLocation.of_site() function, which makes it possible to obtain an EarthLocation object from an observatory name. The data in the JSON-file contains (among others) fields called "name" and "aliases", and in quite a few cases the "name" value is duplicated in "aliases", for example:

astropy-data/coordinates/sites.json

Lines 44 to 57 in 84deb2c

	"lick": {
	"source": "IRAF Observatory Database",
	"elevation": 1290,
	"name": "Lick Observatory",
	"longitude_unit": "degree",
	"latitude_unit": "degree",
	"latitude": 37.343333333333334,
	"elevation_unit": "meter",
	"longitude": 238.36333333333332,
	"timezone": "US/Pacific",
	"aliases": [
	"Lick Observatory"
	]
	},

Prior to astropy/astropy#12721 this duplication served a purpose because names usable with the EarthLocation.of_site() function had to be listed in "aliases". That pull request updated the code so that "name" also gets inspected and the duplicates could be removed, but it can't be done immediately because not everybody can be reasonably expected to already be using a version of astropy with the updated code. This issue serves as a reminder that the duplicates should be removed eventually.

A related question, the answer to which is also included in the text @pllim quoted, is whether or not https://github.com/astropy/astropy-data/blob/gh-pages/tests/test_sites_json.py should contain a test that would ensure that different observatories would have different names.

[dlakaplan]

On the suggestion of @hamogu , I added a test in https://github.com/dlakaplan/astropy-data/blob/duplicatecheck/tests/test_sites_json.py. Right now this raises an error if it finds a duplicate (appropriate for some CI test) but other actions could occur. When I run it locally the only issues I find are not duplications between names/aliases for the same site (those are explicitly allowed) but several sites that include a null string as an alias:
irtf
ohp
sirene

Those seem like they should be fixed and would not affect anything else, but maybe there is a reason to include them?

Any feedback would be welcome.

[eerovaher]

Currently the astropy machinery explicitly discards the empty strings, so if the data file wouldn't contain them to begin with then the astropy machinery could be simplified accordingly.

Another thing to pay attention to is that the astropy machinery uses lowercase versions of the strings to do the lookups, which means that aliases that differ only by capitalization are actually duplicates that should be cleaned up.

[dlakaplan]

I updated my test to ignore empty strings, but it seems like they are not necessarily discarded by astropy:

In [3]: import astropy.coordinates
In [4]: astropy.coordinates.EarthLocation.of_site("sirene")
Out[4]: <EarthLocation (4575207.58706027, 439490.22322285, 4408855.73665184) m>
In [5]: astropy.coordinates.EarthLocation.of_site("")
Out[5]: <EarthLocation (4575207.58706027, 439490.22322285, 4408855.73665184) m>

So you can query for a null site, and it returns the same as sirene.

[eerovaher]

My previous comment about empty strings described the main branch of astropy, but I didn't make that clear. astropy explicitly discards the empty strings since version 5.1, but older versions, including the current LTS, do not. Getting rid of the empty strings in the data file would fix what I consider to be a bug for LTS and allow for simplifying the code in astropy main, so it's beneficial in both cases.