Cleanup: Remove Property.from_string and to_string

[tbennun]

It is redundant with to_json and from_json.

[HarshvMahawar]

I would like to take up this issue but as Im new here can you help me with the thinking process, thanks

[tbennun]

The Property class has methods called to_string and from_string. These are now redundant with the to and from json methods that already exist in the codebase. It should be possible to remove those methods, as well as set_property_from_string.

You just need to make sure that no existing system depends on it. If you have any questions, feel free to ask.

[HarshvMahawar]

after restructuring the file properties.py, Is there any particular test file to run to check the dependency of property.to_string or from_string or will have to just run the pytest tests/. Also, you being an expert open-source developer can you suggest any good tool fit for open-source development especially spcl org. thanks :)

[tbennun]

You should run the tests with pytest. After making a pull request, we will run the entire test suite as part of our continuous integration (CI).

We mostly use Visual Studio Code for development. There is also a nice DaCe plugin for it too.

[luca-patrignani]

Hi, I'm new to the project and I started working on this issue. I am noticing that lots of code depends on from_string method. In particular dace.serialize.from_json depends on it and this function is used by from_json of most Property's subclasses.

[tbennun]

@luca-patrignani Thank you for starting to work on this!

Individual Property subclasses may define their own static methods to convert from a string to themselves. So keeping, say, CodeProperty.from_string is fine. What we want to see is removing from_string and to_string from the general Property API, which will not force users to write another deserialization method apart from JSON.

The usage in dace.serialize.from_json is not the same as the properties. It is checking for a method with that name on a known_dtype, so it is safe to keep or remove. I don't think this code is ever called: https://app.codecov.io/gh/spcl/dace/blob/master/dace%2Fserialize.py#L124

Also, as said in my previous comment, this change will also result in free functions such as set_property_from_string to be safely removed.

[NNhanptnk]

Hello !

I want to join helping for this issue as well. Can I join ?
I am relatively new. Where would you recommend me reading before jumping right in ? Thank you !

[tbennun]

@NNhanptnk I recommend starting by reading the documentation. The links I give in the two above comments are good starting points.