ee.Dictionary to ee.FeatureCollection

[fitoprincipe]

#368 (review)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.96%. Comparing base (a45bdb1) to head (5e6f1b9).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
+ Coverage   90.85%   90.96%   +0.10%     
==========================================
  Files          27       27              
  Lines        1663     1683      +20     
  Branches       78       80       +2     
==========================================
+ Hits         1511     1531      +20     
  Misses        130      130              
  Partials       22       22              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[12rambau]

I would use the same pattern as the other to maintain one single return statement:

Suggested change

	return ee.FeatureCollection([ee.Feature(None, self._obj)])
	features = ee.list(ee.Feature(None, self._obj))

[12rambau]

This leads to 2 very differnet ways of treating the dictionnary keys and I think it's more confusing than helping. At the end of the day, the use case is when the keys of the dict are the future system:index of the features. The case were the keys are simply treated as column names is so trivial (f = ee.Feature(, None, self._obj)) that I think it should not be accepted as an input of this function.

Narrowing down the use cases will also make it easier to understand the first sentence of the docstring "The keys will always be stored in system:index column."

[12rambau]

Alternative building blocks. I think setting all the props at once makes it easier to understand what the function is doing. As there is no line gain, it's up to you to decide which one is better

Suggested change

	key = ee.String(key)
	feat = ee.Feature(None, {"system:index": key})
	return feat.set(ee.Dictionary(value))
	index = {"system:index": ee.String(key)}
	props = ee.Dictionary(value).combine(index)
	return ee.Feature(None, props)

[12rambau]

Alternative building blocks:

Suggested change

	key = ee.String(key)
	feat = ee.Feature(None, {"system:index": key})
	value = ee.List(value)
	keys = ee.List.sequence(1, value.size())
	keys = keys.map(lambda k: ee.String("value_").cat(ee.Number(k).toInt()))
	properties = ee.Dictionary.fromLists(keys, value)
	return feat.set(properties)
	index = {"system:index" : ee.String(key)}
	values = ee.List(value)
	columns = ee.List.sequence(1, value.size())
	columns = columns.map(lambda k: ee.String("value_").cat(ee.Number(k).toInt()))
	props = ee.Dictionary.fromLists(columns, values).combine(index)
	return ee.Feature(None, props)

[12rambau]

any reason why the values index starts at 1 instead of 0 ?

[12rambau]

The long description needs to be improved so the user can understand exactly what this function is doing with both the keys and the values. In the current implementation, the default behaviour (1 feature, non-collection dType) is not described.

[12rambau]

We now have full support of intersphinx so let's use it as much as we can:

Suggested change

	"""Convert a ee.Dictionary to a ee.FeatureCollection with no geometries (table).
	"""Convert a :py:class:`ee.Dictionary` to a :py:class:`ee.FeatureCollection` with no geometries (table).