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).
  • 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.
• Run make lint on the project, ensure it passes
  • 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)
  • Review all TODOs, convert to issues wherever possible
  • 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 <#353 (comment)>, 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.