is_subgroup() modifications in SpaceGroup and PointGroup

[kaueltzen]

Summary

Resolves #3937 .

SpaceGroup

• enabled isomorphic group subgroup relationships, added test
• Constrained

  pymatgen/src/pymatgen/symmetry/groups.py

  Line 528 in 0234182

  if len(supergroup.symmetry_ops) < len(self.symmetry_ops):

  to non-klassengleiche group subgroup relationships so cases like assert SpaceGroup("Fm-3m").is_subgroup(SpaceGroup("Pm-3m")) pass, added test
• fixed bug introduced by me in SpaceGroup changes #3859 (Apologies!) of wrong short Hermann Mauguin symbol computation for multiple trigonal groups
• modified point_group entries of symm_data.json to be possible symbol attributes of PointGroup (also see comment below)

PointGroup

• added crystal_system attribute to PointGroup
• enabled init from other settings than in database and from full symbol, added test
• added notes in docs that axes of PointGroup object may differ from given int_symbol in init / given space group symbol in from_space_group as currently only one setting per point group is available
• added crystallographic direction issue check for is_subgroup of PointGroup -> a NotImplementedError is now raised if the blickrichtungen / crystallographic directions of the crystal systems of the two groups differ

[kaueltzen]

Hey @shyuep @mkhorton @janosh this is ready to be reviewed.

[janosh]

looks great! thanks @kaueltzen. just a minor suggestion

[janosh]

typo: Only trivial (subsets->supergroup)...

maybe extract these duplicate messages into a trivial_group_warning format string with a which_set parameter that you fill with subgroup/supergroup in def is_(sub|super)group

[kaueltzen]

Hi, I'm not sure what you mean @janosh . A function like this?

def trivial_group_warning(subgroup: str, supergroup: str) -> str:
    return (
        f"This is not fully functional. Only trivial subsets are tested right now."
        f"This will not work if the crystallographic directions of subgroup "
        f"{subgroup} and supergroup {supergroup} are different."
    )

that is called in is_(sub|super)group?

warnings.warn(trivial_group_warning(subgroup=self.symbol, supergroup=supergroup.symbol))

[kaueltzen]

Hey @janosh can you maybe elaborate on your comment from Aug 2? Thanks!

[kaueltzen]

Hi, I would like to have some more opinions on this: With the current changes, all "point_group" entries in symm_data.json and symm_ops.json are possible symbols of PointGroup. This means that the setting of a SpaceGroup object and its point_group attribute may be different, for example, "P-4m2" and "-42m" (as there is no "-4m2" setting of this point group in symm_data.json currently.
Therefore, SpaceGroup objects of the same point group always have the same point_group attribute.
Also, this enables quick comparisons like https://github.com/kaueltzen/pymatgen/blob/e1ec53387a22012936afbf693fcd623a405db690/src/pymatgen/symmetry/groups.py#L584 but of course one could also agree on a "non-standard" point_group attribute in agreement with the space group setting and adapt respective code snippets
if len(supergroup.symmetry_ops) < len(self.symmetry_ops) and PointGroup(supergroup.point_group).symbol != PointGroup(self.point_group).symbol:

[janosh]

is this still waiting on external input? looks like this PR went stale so maybe need to ping someone?

[kaueltzen]

Hey @janosh , originally, i was hoping for input from you, @mkhorton or @shyuep . However, if there are no strong opinions on this, these changes can be merged.