materialsproject / api

New API client for the Materials Project
https://materialsproject.github.io/api/
Other
105 stars 34 forks source link

API doc is unclear #834

Open shyuep opened 11 months ago

shyuep commented 11 months ago

@munrojm @janosh

It is rather difficult to find examples on how to do most API queries. Getting to the API page requires multiple clicks on links and some of those links are circular.

Further, the difference between search and _search is rather subtle. For instance

mpr.materials.search(nelements=2) fails because nelements is apparently not a keyword, despite the documentation saying it is.

But mpr.materials._search(nelements=2) inexplicably works.

  1. General python convention is that an underscore in front of a method name is a private function and should not be part of an exposed method API, i.e., developers have the right to modify this method at will with no regard to backwards compatibility. So the documentation really shouldn't recommend people use _search as the underlying method.
  2. It is unclear why nelements does not work for search.
  3. I suggest modifying _search to something else, like query, or at least, search_advanced.
munrojm commented 11 months ago

The difference is that _search can accept parameters defined by the raw API schema shown here. The arguments of search are defined by us, and let us simplify how some of the querying is done for a more streamlined user experience. All of the parameters that can be passed are properly type hinted and documented in the docstring of search for users to see. The intention is that people should just use search.

Renaming _search is definitely something we can do. Also happy to change things in the documentation, including the examples page to make things clearer. Are there any specific changes related to documenting the input arguments of search you feel we should make?

shyuep commented 11 months ago

I guess the basic question of what parameters can a method take? E.g. why does nelement not work with search but works with _search?