Mapper method alias_of extracts alias wrongly

[CasperWA]

In the following line:

optimade-python-tools/optimade/server/mappers/entries.py

Line 121 in 790ab7d

return {alias: real for real, alias in cls.all_aliases()}.get(field, field)

real, alias needs to be alias, real for the alias_of() method to work.

[ml-evs]

Hmmm, if this needs to be changed then presumably alias_for(...) does too?

How does this relate to the test case here? (specifically line 31 vs 37, which seems to me like this is working as intended?)

optimade-python-tools/tests/server/test_mappers.py

Lines 22 to 39 in 790ab7d

	def test_property_aliases(mapper):
	class MyMapper(mapper(MAPPER)):
	PROVIDER_FIELDS = ("dft_parameters", "test_field")
	LENGTH_ALIASES = (("_exmpl_test_field", "test_field_len"),)
	ALIASES = (("field", "completely_different_field"),)
	mapper = MyMapper()
	assert mapper.alias_for("_exmpl_dft_parameters") == "dft_parameters"
	assert mapper.alias_for("_exmpl_test_field") == "test_field"
	assert mapper.alias_for("field") == "completely_different_field"
	assert mapper.length_alias_for("_exmpl_test_field") == "test_field_len"
	assert mapper.length_alias_for("test_field") is None
	assert mapper.alias_for("test_field") == "test_field"
	assert mapper.alias_of("dft_parameters") == "_exmpl_dft_parameters"
	assert mapper.alias_of("test_field") == "_exmpl_test_field"
	assert mapper.alias_of("completely_different_field") == "field"
	assert mapper.alias_of("nonexistent_field") == "nonexistent_field"

[CasperWA]

I don't understand how that works.
If you just look at the line, as it is now it assumes that all_aliases is a list of tuples with 2 entries. The first being the real field name, and the second being the aliased field name (i.e., the OPTIMADE field name).
At least that's how I understand real and alias.
In your example, I understand that you're setting up ALIASES the other way around, i.e., the first being the OPTIMADE "alias" and the second being the database's "real" field name?
So again, I don't understand how that test works succeeds... 😅

[CasperWA]

But maybe it's always been supposed to be the other way around? Hmmm.. The definition of real and alias confuses me. Along with the documentation descriptions. Especially when analyzing your test there.

[CasperWA]

Okay, so I've just fixed this by using alias_for instead. However, it still seems to me that these two functions are switched as to what they state in the doc-strings. Can we maybe do better function names or establish better what is meant by alias and real?

[ml-evs]

Okay, so I've just fixed this by using alias_for instead. However, it still seems to me that these two functions are switched as to what they state in the doc-strings. Can we maybe do better function names or establish better what is meant by alias and real?

Yeah, sounds sensible.

alias_for -> alias_for_optimade_field and alias_of -> alias_of_optimade_field?

Then we can keep the old function names as... aliases... for the new ones 🙃

[ml-evs]

Or is it the arbitrary of/for distinction that is the problem? In which case...

alias_for -> alias_for_optimade_field and alias_of -> alias_for_data_field?

[CasperWA]

I like the latter more. But I still feel they are a bit wishy-washy.

How about alias_for -> db_field_for_optimade_field and alias_of -> optimade_field_for_db_field?
Or loose field and have the more vague db_for_optimade/optimade_for_db, but then maybe to instead of for?

[CasperWA]

Or maybe use from, as what you really want to use these functions for is to retrieve the other one that you have. I.e., I want the OPTIMADE field from a DB field, and vice versa..?

[ml-evs]

We also have to consider that non-OPTIMADE provider fields can be aliased so maybe need an even more general name... how about get_response_field_name(...), get_backend_field_name(...)?

[CasperWA]

I like that!
The only weak part of those names is the response part. But I understand why you have it. Could we have optimade instead there or are you trying to avoid getting optimade into the method names as to not be too self-aware (or something)? :)

[ml-evs]

get_api_field_name? I'm fine with optimade though

[CasperWA]

optimade 🎉 hehehe (mostly also because api is very general. One could argue it could be the DB API ... )

[ml-evs]

Hey @CasperWA, I'm going to self-assign and rename these functions as we discussed.