Wyckoff_position: from (...) should be classmethods/staticmethods

[Somerandomguy10111]

The class Wyckoff_position has several constructor methods e.g.

• from_group_and_letter
• from_dict

When you use any method of the class, an IDE will automatically assume the first argument of the method to be "self". Because self will be passed automatically when it is called on an instance of the class, this makes a lot of sense. But this will never be the case for constructor methods.

Hence when you pass e.g. group = 65, letter = a to the method i.e. call

pos = Wyckoff_position.from_group_and_letter(65, 'a')

the IDE warns you: "Expected type Wyckoff_position got int instead". This is because it assumes the first argument to be self i.e. an instance of the class. This is simple to fix: These constructor methods can be made into classmethods or staticmethods using the corresponding decorator:

@classmethod
    def from_group_and_letter(group, letter, dim=3, style='pyxtal', hn=None):

[qzhu2017]

@Somerandomguy10111 Thanks for your comments. This class was created by my student @scottfredericks long time ago for his master project. It would be a great idea to redesign it in a more pythonic way. However, I am a bit busy at some other stuff. If possible, would you please try this modification and send us a pull request. After you are done with the implementation, you can just run python pyxtal/test_all.py to test if it passes all test cases.

[Somerandomguy10111]

Hi, I opened a pull request that fixes this issue. I ran the tests as suggested and found them to pass without issue.

As an aside: This change also has the added benefit, that a user can now subclass the Wyckoff_Position class e.g. to add his own functionalities, and use from_group_and_letter etc. to construct the subclass instance, rather than a Wyckoff_Position instance.

[qzhu2017]

@Somerandomguy10111 Thanks. I just merged it to the main branch :)