Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't remove Atoms.specie #814

Open
pfebrer opened this issue Aug 1, 2024 · 3 comments
Open

Don't remove Atoms.specie #814

pfebrer opened this issue Aug 1, 2024 · 3 comments

Comments

@pfebrer
Copy link
Contributor

pfebrer commented Aug 1, 2024

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
Copy link
Owner

zerothi commented Aug 1, 2024

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
Copy link
Contributor Author

pfebrer commented Aug 1, 2024

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
Copy link
Owner

zerothi commented Aug 1, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants