search

wtbarnes / fiasco

Python interface to the CHIANTI atomic database
http://fiasco.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
20 stars 15 forks source link

Type hints for functions #281

Open jwreep opened 2 months ago

jwreep commented 2 months ago

The return statements in functions in fiasco can be quite varied, both in terms of the units and the data types/structures. There are still a good number of functions without type hints, so I think it might be worth going through and adding type hints to improve readability of the code.

Any thoughts?

wtbarnes commented 2 months ago

Just to clarify, by type hints do you mean adding unit annotations to the inputs and outputs or add type hints for the data types of the inputs and outputs (eg list, int, etc)?

I ask because I'm all in favor of the former, but I'm very wary of the latter.

jwreep commented 2 months ago

Both. Why are you wary of the latter?

wtbarnes commented 2 months ago

I find the addition of type hints/annotations makes Python code more cluttered and less readable. The value added by having these annotations does not seem to justify the cost. However, I recognize this is largely personal preference and I recognize I'm increasingly in the minority.

If you find it helpful, it's worth considering. Did you have a particular function/method in mind where this would be useful?

jwreep commented 2 months ago

I don't have particularly strong opinions one way or other, so I'm not going to push about it. Certainly units would be helpful though.

I was looking at the ion collections (thinking of working on #232) and noticed that the functions there are missing type hints. E.g. https://github.com/wtbarnes/fiasco/blob/83eb4d2b47bb3484d91f2d7c4c8a8d9f5c59e982/fiasco/collections.py#L83 https://github.com/wtbarnes/fiasco/blob/83eb4d2b47bb3484d91f2d7c4c8a8d9f5c59e982/fiasco/collections.py#L213-L214