search

zerothi / sisl

Electronic structure Python package for post analysis and large scale tight-binding DFT/NEGF calculations
https://zerothi.github.io/sisl
Mozilla Public License 2.0
198 stars 60 forks source link

Don't remove `Atoms.specie` #814

Open pfebrer opened 3 months ago

pfebrer commented 3 months ago

When running code with the main branch, I get:

(dep:0: SislDeprecation: specie is deprecated, use species instead. [>=0.15] [removed in 0.16])

I think it is a bad idea to make specie not work for sisl>=0.16. It will break lots of codes unnecesarily. We could simply not document it or document it with a big warning stating that it is not the desired way of using it. In general I think we should start to care more about backward compatibility, now that there is a substantial user base.

zerothi commented 3 months ago

Yeah, I have these all over.

The main idea is that users react to this and start converting their code. We have to deprecate them sometimes. And since we don't know when 0.16 will be released, I think it is safe to say we can say something like that! It will at least inform users of something breaking later down the road, when 0.16 hits, we just have to make a decision on whether to post-pone the deprecation warning or actually delete it.

I would prefer that we are aggressive here so it doesn't hold up development. For this particular case it isn't a problem, but for some where it changes the API, it is more important to make people adapt as it holds back other things.

pfebrer commented 3 months ago

As you say, it may be important in some cases, but removing specie won't help with anything.

The main idea is that users react to this and start converting their code.

I understand that, but sometimes users have not written the code and don't understand what is in it, they are just using it.

Also, too many warnings for superfluous things like this might make people worry (with a reason), that the code they develop with sisl might become obsolete in 2 years and will become unusable for other people unless they are still around to maintain it.

zerothi commented 3 months ago

What we aim with sisl is to develop towards an API stable solution (semver). Until then I would say we have some leverage to work under.

The counter argument is that it shows that sisl is being maintained and actively developed. So far very few users come back with these deprecations. I have only experienced a hand-ful.

The current issues we have with API stability and chances to API's are extremely similar, numpy, xarray, matplotlib all have these issues as well, and they are deailing with it to some extend.

When I raise these issues with larger tools they acknowledge, but reiterate what I say, it shouldn't hold back development AND, this is open-software, nobody gets paid to do it...

I think we have to deprecate things. And rightfully so should we do it when it makes sense.

As you say, it may be important in some cases, but removing specie won't help with anything.

sure it will, specie is not even an english word (it is actually a Danish cookie) ;)

Having it, without deprecation warnings will make users get used to using certain things, which we don't want them too. The API can always be discussed... We just want users to give feedback!