Enable to add similar elements

[jkpnen]

Solves #194

[jkpnen]

Tests will not pass (after the latest commit e536dcd) if not sorted(orig_elems). Why is that and should it be sorted?

https://github.com/JYU-IBA/potku/blob/96cf37bf3be94874f9b4f96a20022abcceab0ef7/tests/unit/test_element.py#L64-L82

[tpitkanen]

Tests will not pass (after the latest commit e536dcd) if not sorted(orig_elems). Why is that and should it be sorted?

https://github.com/JYU-IBA/potku/blob/96cf37bf3be94874f9b4f96a20022abcceab0ef7/tests/unit/test_element.py#L64-L82

__lt__ is the less-than comparison. The new commit broke the isotope ordering/sorting as I noted here. The test should work as-is after the commented thing is fixed.

[tpitkanen]

I checked this pull request again while merging the open PRs and noticed the following:

• __init__: sequence defaults to 0 -> the sequence number is visible for each element
• Does the if element_str == 'SUM': check work for the new style of sum spectra?
• __str__ is not updated
  • there is a corresponding from_string method, so the following must always hold: elem == Element.from_string(str(elem))
  • changing this will probably cause issues in places where it's used
  • the __lt__ comparison method relies on this to compare elements that both have a sequence
    • comparison is currently subtly broken: __eq__ correctly compares non-None sequences, but __lt__ doesn't
• if self.symbol is None and other.symbol is not None: and if self.symbol is not None and other.symbol is None: are not needed because there's already if self.symbol != other.symbol: above them
  • even if the check weren't there, the symbol parameter is mandatory so there shouldn't be a situation where it's None
• Outdated docstring:
  • __init__
  • __str__
  • __lt__
  • get_prefix

---

More importantly, I reconsidered the risk/reward ratio of this feature. While being able to add similar elements is useful, the Element class is used in many places (around 100 non-test locations according to Find Usages, of which from_string accounts for 24 occurrences). Any of these places could break with this change.

Even if the code doesn't break, the effects of the new sequence member variable need to be considered in each of the places Element is used.

I don't think this feature should be added after all. It's too much work and code complexity for its benefits.

(Ping for the corresponding issue: #194)

[tpitkanen]

@mlaitin: Just two types of same element would be enough (SCT/REC).

This feature should be simpler/safer to implement this way.

194