Make SOAP initialization more intuitive w.r.t. species lists

[max-veit]

Fix #350

In particular, merge the expansion_by_species_method and global_species keys in the Python interface, so the confusion that led to the aforementioned issue doesn't happen again.

Tasks before review:

• [ ] Add tests, examples, and complete documentation of any added functionality
  • [ ] Make sure the autogenerated documentation appears in Sphinx and is formatted correctly (ask @max-veit if you need help with this task).
  • [x] Make sure the examples run (!) and produce the expected output
  • [ ] For bugfix pull requests, list here which tests have been added or re-enabled to verify the fix and catch future regressions as well as similar bugs elsewhere in the code.
• [x] Run make lint on the project, ensure it passes
  • [x] Run make pretty-cpp and make pretty-python, check that the auto-formatting is sensible
  • [ ] Review variable names, make sure they're descriptive (ask a friend)
  • [x] Review all TODOs, convert to issues wherever possible
  • [x] Make sure draft, in-progress, and commented-out code is moved to its own branch
• [ ] Merge with master and resolve any conflicts

[agoscinski]

Don't you think expansion_by_species_method is the better variable name to keep? Open for any other naming, species_list is just a bad name for variable which can also be a string

[max-veit]

No, I deliberately chose a different name to make the switch to the new behaviour clear and be able to warn code trying to use the old options. Also, I really wasn't a fan of the old name; too verbose and technical.

As for the new name, why is it a problem that you can pass a string instead of a list?

SphericalInvariants(..., species_list="structure wise", ...)

reads very well to me; it just says "the species list is defined structure wise".

[ceriottm]

duck typing at its best ^_^

On Mon, 10 May 2021 at 17:48, Max Veit @.***> wrote:

No, I deliberately chose a different name to make the switch to the new behaviour clear and be able to warn code trying to use the old options. Also, I really wasn't a fan of the old name; too verbose and technical.

As for the new name, why is it a problem that you can pass a string instead of a list?

SphericalInvariants(..., species_list="structure wise", ...)

reads very well to me; it just says "the species list is defined structure wise".

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cosmo-epfl/librascal/pull/353#issuecomment-836865862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIREZ4KACBAWCFOYBPA4PDTM756HANCNFSM44RL5ARA .

[agoscinski]

find it confusing when variable name contains a data type and also accepts other data types. I would go with something as species_storage but its not so important, its your call.