pyiron / pyiron/structuretoolkit
Documentation needed in`common.helper`
@jan-janssen is already working on this.
Since Sep 5, 2023.
- Dominant language
- Python
- Stars
- 8
- Forks
- 1
- Avg merge
- 1d 28m
- Merged PRs (30d)
- 10
Description
I think common.helper.get_species_indices_dict is not behaving as intended, but it's hard to tell with neither docstring nor test. Note that this dictionary only couples species and their index of first appearance.
EDIT:
Ok, common.helper.get_species_indices_dict is doing what it is supposed to by returning the index of the first occurrence of each species. The source of my confusion was that this usage of indices as a numeric formulation of the species information in Atoms.get_chemical_symbols rather than the integer range $[0, N_\mathrm{atoms}-1]$.
The ase Atoms docs consistently use indices to mean exactly this 0..N-1 range, while the use of indices here is a holdover from pyiron-specific terminology.
I don't think it's the end of the world to introduce or re-define terms compared to ASE. However, if and when we do that, it does become important to have documentation regarding the differences. (I mean, we need documentation anyhow, but it becomes especially important in such cases.)
Right now the four indices-related methods in common.helper (get_species_indices_dict, get_structure_indices, select_index, set_indices) are sorely in need of documentation.
At the same time, we could consider alternative nomenclature that doesn't conflict with the ASE meaning for indices -- but if we can't think of anything, then documenting the difference is sufficient.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.