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

Depreciate functions not in connection class #86

Closed
chrisbeardy opened this issue Sep 5, 2019 · 6 comments · Fixed by #128
Closed

Depreciate functions not in connection class #86

chrisbeardy opened this issue Sep 5, 2019 · 6 comments · Fixed by #128

Comments

@chrisbeardy
Copy link
Collaborator

A question, do you think it is worth depreciating old functions in "ads.py" that are not part of the connection class or not required for linux essentially forcing people to use the Connection class. Then rewriting the documentation to clear up the usage, esspecially for connecting in linux.

I think this would help clear up the repository, make it easier to navigate and use for newcomers and means you don't have to duplicate functions. Comes at the cost of losing backwards compatibility but may be worth doing and upgrading to V4. Point older users to older releases.

Let me know what you think and I'll put in a PR.

I'm looking at adding support for structures of multiple type and it took me a little while to clock on to the duplicate functions and how they are working.

@stlehmann
Copy link
Owner

I still have a project running that relies on the old function calls and I can imagine that some others might as well. Since there is the Connection class interface I strongly recommend using this because it is much more intuitive.

I guess it can be a bit confusing having the old function calls and the "new" Connection-class interface, especially in the documentation, as you pointed out.

So it might be a good idea to get rid of the legacy functions comes a new major version.

@chrisbeardy
Copy link
Collaborator Author

Yes maybe for the next major version. After any other functionality is added. May be worth thinking about python2 compatibilty for that release too.

Hopefully any old projects won't be making use of any new functionality therefore can just continue to use an old version of pyads.

@chrisbeardy
Copy link
Collaborator Author

I'll pop in a pull request then can let you decide when/if to accept at a later time as it shouldn't create a conflict at any time if no new functionality is added to not the Connection class. Will close this issue for now.

@stlehmann stlehmann reopened this Oct 4, 2019
@stlehmann
Copy link
Owner

I would like to add the deprecation warnings in the next version of pyads. Did you draft any PR for this, yet ... no pressure, just asking ;) ? Also I agree with you that it's about time to drop the py2 support for the next major version.

@chrisbeardy
Copy link
Collaborator Author

sorry not yet, i was working on another PR for black formatting, adding some #fmt:off and #fmt:on flags then reformatting all the source files so that it is easier tokeep to black formatting in the future. I'll pop that one in shortly. Then can do the PR for depreciation too if you want?

@stlehmann
Copy link
Owner

Oh man, so many changes...But I feel it' s a good thing to finally have all code "blacked". Thx for this. If you also would like to do the PR for depeciation you're very welcome to do so.

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

Successfully merging a pull request may close this issue.

2 participants