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

Final pass over the deprecated public modules #79

Merged
merged 40 commits into from
Oct 13, 2024

Conversation

pavyamsiri
Copy link
Contributor

@pavyamsiri pavyamsiri commented Oct 12, 2024

This should fully close issue #48. See the list at the end of the comment to see all of the modules that are public but are going to be deprecated. I found them by looking at the top most comment that says that they will be deprecated in v2.0.0 in the actual scipy git repo. Tell me if I should just add the list to my original issue instead.

This PR should address the rest of the modules not stubbed out by #54.

Problems with merging

It seems that @jorenham has already tried to stub out some of the deprecated modules already so there are conflicts that need to be addressed.

List of all deprecated public modules

There are 109 such modules in total.

  • signal.ltisys
  • signal.spectral
  • signal.waveforms
  • signal.bsplines
  • signal.fir_filter_design
  • signal.signaltools
  • signal.wavelets
  • signal.lti_conversion
  • signal.filter_design
  • signal.windows.windows
  • sparse.coo
  • sparse.data
  • sparse.sparsetools
  • sparse.dok
  • sparse.csr
  • sparse.compressed
  • sparse.lil
  • sparse.spfuncs
  • sparse.extract
  • sparse.base
  • sparse.sputils
  • sparse.construct
  • sparse.bsr
  • sparse.dia
  • sparse.csc
  • sparse.linalg.interface
  • sparse.linalg.matfuncs
  • sparse.linalg.dsolve
  • sparse.linalg.eigen
  • sparse.linalg.isolve
  • constants.codata
  • constants.constants
  • interpolate.fitpack2
  • interpolate.dfitpack
  • interpolate.fitpack
  • interpolate.polyint
  • interpolate.interpolate
  • interpolate.ndgriddata
  • interpolate.rbf
  • optimize.linesearch
  • optimize.minpack
  • optimize.slsqp
  • optimize.tnc
  • optimize.cobyla
  • optimize.optimize
  • optimize.lbfgsb
  • optimize.zeros
  • optimize.minpack2
  • optimize.nonlin
  • optimize.moduleTNC
  • linalg.matfuncs
  • linalg.decomp_lu
  • linalg.decomp_svd
  • linalg.decomp_qr
  • linalg.decomp_schur
  • linalg.decomp
  • linalg.basic
  • linalg.misc
  • linalg.special_matrices
  • linalg.decomp_cholesky
  • odr.models
  • odr.odrpack
  • misc.common
  • misc.doccer
  • fftpack.realtransforms
  • fftpack.helper
  • fftpack.pseudo_diffs
  • fftpack.basic
  • io.idl
  • io.mmio
  • io.harwell_boeing
  • io.netcdf
  • io.matlab.mio_utils
  • io.matlab.mio4
  • io.matlab.mio5_params
  • io.matlab.mio
  • io.matlab.mio5_utils
  • io.matlab.streams
  • io.matlab.byteordercodes
  • io.matlab.mio5
  • io.matlab.miobase
  • io.arff.arffread
  • stats.kde
  • stats.morestats
  • stats.mvn
  • stats.mstats_extras
  • stats.biasedurn
  • stats.stats
  • stats.mstats_basic
  • ndimage.morphology
  • ndimage.fourier
  • ndimage.interpolation
  • ndimage.filters
  • ndimage.measurements
  • integrate.vode
  • integrate.dop
  • integrate.odepack
  • integrate.quadpack
  • integrate.lsoda
  • special.specfun
  • special.add_newdocs
  • special.basic
  • special.spfun_stats
  • special.sf_error
  • special.orthogonal
  • spatial.qhull
  • spatial.kdtree
  • spatial.ckdtree
  • spatial.transform.rotation

@pavyamsiri pavyamsiri changed the title Potentially final pass over the deprecated public modules Final pass over the deprecated public modules Oct 12, 2024
@jorenham
Copy link
Owner

Wow great, thanks for this!

In the future you could avoid such conflicts by splitting it up by e.g. package. Just to be clear; I'm not saying you should split this one up or anything, but it's just something I've been trying to do myself 🤷🏻‍♂️

@jorenham
Copy link
Owner

I found them by looking at the top most comment that says that they will be deprecated in v2.0.0 in the actual scipy git repo. Tell me if I should just add the list to my original issue instead.

At this early development stage there's no need to worry about formalities too much.
So as far as I'm concerned you can do whatever you think is best in this case 👌🏻

@jorenham
Copy link
Owner

It seems that @jorenham has already tried to stub out some of the deprecated modules already so there are conflicts that need to be addressed.

My earlier changes were mostly directed to fixing the failing stubtest, which has some unavoidable overlap with your work on the deprecations.
The merge conflicts look very minimal, and you changes are clearly an improvement in those cases.

@jorenham
Copy link
Owner

jorenham commented Oct 12, 2024

Oh and quick little DRY idea I had in the case of deprecated classes, is to e.g.

from typing_extensions import deprecated
from ._ckdtree import cKDTree as _cKDTree

@deprecated("Lorem ipsum")
class cKDTree(_cKDTree): ...

I believe that this is "correct enough", at least as far as stubtest and pyright concerned

@pavyamsiri
Copy link
Contributor Author

Wow great, thanks for this!

In the future you could avoid such conflicts by splitting it up by e.g. package. Just to be clear; I'm not saying you should split this one up or anything, but it's just something I've been trying to do myself 🤷🏻‍♂️

That's a fair suggestion. I think I thought it would be too many PRs but in hindsight the monolithic nature of this PR is also a problem in and of itself.

Stubbing out `__getattr__` may confuse type checkers and in the case of
these two files is entirely unnecessary as they do not export anything
anyway.
@jorenham
Copy link
Owner

the monolithic nature of this PR is also a problem in and of itself.

oh don't worry it about it; if this woould've been e.g. a numpy PR then it would've been a different story, but at this point in scipy.stubs I really don't mind

@pavyamsiri pavyamsiri force-pushed the deprecated-public-modules-p2 branch from 2e19d35 to 6639e5d Compare October 13, 2024 01:34
@pavyamsiri
Copy link
Contributor Author

Oh and quick little DRY idea I had in the case of deprecated classes, is to e.g.

from typing_extensions import deprecated
from ._ckdtree import cKDTree as _cKDTree

@deprecated("Lorem ipsum")
class cKDTree(_cKDTree): ...

I believe that this is "correct enough", at least as far as stubtest and pyright concerned

Ah this is very helpful. Should cut down on the file size as well.

I tried to shadow the inheritance structure at first but I got warnings about using deprecated classes from a lint so I ended up having to stub out every single method and inherited method as well.

I really should have just tried what you just did though.

@jorenham
Copy link
Owner

Maybe it could help to have the deprecated OptimizeResult classes inherit from typing_extensions.Any, so that typecheckers will allow accessing any attribute, instead of none?

And I spotted one or two deprecated OptimizationWarning without a base class, otherwise it won't be usable in e.g. warnings.catch_warnings

@pavyamsiri
Copy link
Contributor Author

Maybe it could help to have the deprecated OptimizeResult classes inherit from typing_extensions.Any, so that typecheckers will allow accessing any attribute, instead of none?

And I spotted one or two deprecated OptimizationWarning without a base class, otherwise it won't be usable in e.g. warnings.catch_warnings

It doesn't seem like you can inherit typing_extensions.Any. mypy seems to think that you can't at least.

@jorenham
Copy link
Owner

jorenham commented Oct 13, 2024

It doesn't seem like you can inherit typing_extensions.Any. mypy seems to think that you can't at least.

I believe it's a basedmypy thing.
You could supress that error with # type: ignore[no-subclass-any]


edit:

it's a basedmypy thing:
https://kotlinisland.github.io/basedmypy/error_code_list3.html#don-t-subclass-any-no-subclass-any

@jorenham
Copy link
Owner

I kinda like your commit message style btw; maybe I'll adopt it here myself 🤔

@pavyamsiri
Copy link
Contributor Author

I kinda like your commit message style btw; maybe I'll adopt it here myself 🤔

Thanks! Although I am not really too consistent with it. I mostly got it from looking at other repos, so I don't have a good grasp of the exact style.

@jorenham
Copy link
Owner

Ready to merge?

@pavyamsiri
Copy link
Contributor Author

Should be all good now.

@jorenham jorenham merged commit 0a99f58 into jorenham:master Oct 13, 2024
19 checks passed
@jorenham
Copy link
Owner

OK great, thanks a lot @pavyamsiri !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scipy.* stubs: improvement Improve or refactor existing annotations topic: deprecation Dealing with deprecated features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What to do with deprecated public submodules
2 participants