Remove py.typed #26158
Remove py.typed #26158
Conversation
sylee957
added
the
pyright
|
✅ Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes.
Click here to see the pull request description that was parsed. |
🟠Hi, I am the SymPy bot. I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it. This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at sympy/sympy-bot#75. The following commits delete files:
If these files were added/deleted on purpose, you can ignore this message. |
|
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) | Change | Before [a00718ba] | After [27bdbeb1] | Ratio | Benchmark (Parameter) |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| - | 69.1±2ms | 44.4±0.3ms | 0.64 | integrate.TimeIntegrationRisch02.time_doit(10) |
| - | 68.1±0.9ms | 43.6±0.4ms | 0.64 | integrate.TimeIntegrationRisch02.time_doit_risch(10) |
| + | 18.1±0.2μs | 30.3±0.4μs | 1.68 | integrate.TimeIntegrationRisch03.time_doit(1) |
| - | 5.38±0.01ms | 2.91±0.02ms | 0.54 | logic.LogicSuite.time_load_file |
| - | 73.1±0.6ms | 28.3±0.2ms | 0.39 | polys.TimeGCD_GaussInt.time_op(1, 'dense') |
| - | 25.8±0.3ms | 16.9±0.1ms | 0.65 | polys.TimeGCD_GaussInt.time_op(1, 'expr') |
| - | 72.9±0.2ms | 28.7±0.09ms | 0.39 | polys.TimeGCD_GaussInt.time_op(1, 'sparse') |
| - | 258±0.9ms | 125±0.9ms | 0.48 | polys.TimeGCD_GaussInt.time_op(2, 'dense') |
| - | 254±3ms | 126±0.9ms | 0.49 | polys.TimeGCD_GaussInt.time_op(2, 'sparse') |
| - | 661±3ms | 371±2ms | 0.56 | polys.TimeGCD_GaussInt.time_op(3, 'dense') |
| - | 664±1ms | 371±2ms | 0.56 | polys.TimeGCD_GaussInt.time_op(3, 'sparse') |
| - | 496±3μs | 288±2μs | 0.58 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense') |
| - | 1.78±0ms | 1.05±0.01ms | 0.59 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense') |
| - | 5.78±0.04ms | 3.05±0.01ms | 0.53 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 443±1μs | 227±0.9μs | 0.51 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense') |
| - | 1.46±0.01ms | 668±6μs | 0.46 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense') |
| - | 4.88±0.01ms | 1.63±0ms | 0.33 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 381±2μs | 207±0.7μs | 0.54 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 2.42±0.02ms | 1.23±0.01ms | 0.51 | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 10.1±0.1ms | 4.33±0.02ms | 0.43 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 360±4μs | 169±1μs | 0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense') |
| - | 2.51±0.03ms | 883±7μs | 0.35 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 9.79±0.2ms | 2.69±0.02ms | 0.27 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 1.03±0.01ms | 424±4μs | 0.41 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 1.72±0.01ms | 499±1μs | 0.29 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.91±0.04ms | 1.79±0.01ms | 0.3 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense') |
| - | 8.36±0.06ms | 1.47±0ms | 0.18 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse') |
| - | 286±0.9μs | 64.0±0.6μs | 0.22 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 3.47±0.04ms | 395±3μs | 0.11 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 4.01±0.02ms | 278±1μs | 0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 7.00±0.04ms | 1.26±0.01ms | 0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense') |
| - | 8.70±0.06ms | 839±3μs | 0.1 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse') |
| - | 4.99±0.05ms | 2.96±0.01ms | 0.59 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| - | 12.1±0.06ms | 6.56±0.03ms | 0.54 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 22.2±0.04ms | 8.96±0.06ms | 0.4 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.28±0.02ms | 879±2μs | 0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 12.6±0.03ms | 7.10±0.1ms | 0.56 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse') |
| - | 103±0.7ms | 25.7±0.2ms | 0.25 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 165±0.8ms | 53.7±0.08ms | 0.32 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 172±0.6μs | 111±0.2μs | 0.65 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 361±2μs | 217±2μs | 0.6 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse') |
| - | 4.29±0.03ms | 838±3μs | 0.2 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 5.35±0.05ms | 377±2μs | 0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse') |
| - | 20.1±0.2ms | 2.85±0.01ms | 0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 23.0±0.3ms | 622±1μs | 0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse') |
| - | 477±0.7μs | 134±1μs | 0.28 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| - | 4.79±0.04ms | 616±4μs | 0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 5.30±0.02ms | 139±2μs | 0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| - | 13.0±0.1ms | 1.28±0ms | 0.1 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 14.0±0.09ms | 140±0.9μs | 0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| - | 134±3μs | 74.8±1μs | 0.56 | solve.TimeMatrixOperations.time_rref(3, 0) |
| - | 252±1μs | 87.6±0.4μs | 0.35 | solve.TimeMatrixOperations.time_rref(4, 0) |
| - | 24.4±0.1ms | 10.2±0.05ms | 0.42 | solve.TimeSolveLinSys189x49.time_solve_lin_sys |
| - | 28.3±0.2ms | 15.7±0.05ms | 0.55 | solve.TimeSparseSystem.time_linsolve_Aaug(20) |
| - | 55.0±0.4ms | 25.4±0.3ms | 0.46 | solve.TimeSparseSystem.time_linsolve_Aaug(30) |
| - | 28.3±0.3ms | 15.2±0.06ms | 0.54 | solve.TimeSparseSystem.time_linsolve_Ab(20) |
| - | 55.3±0.2ms | 24.5±0.1ms | 0.44 | solve.TimeSparseSystem.time_linsolve_Ab(30) |
Full benchmark results can be found as artifacts in GitHub Actions |
|
My sympy-based project NRPy (https://github.com/nrpy/nrpy) uses typehints extensively, and it seems removing py.typed just gave mypy a huge heads up that sympy no longer supports typehints. My CI, which checks |
To be clear SymPy never supported type hints. Just adding a py.typed file without adding the actual type hints is not support for anything. It was a mistake to add it in gh-22337. Quoting from microsoft/pyright#7159:
If we want to add type hints then I have made a start in gh-25103. I think until SymPy can pass a basic check with pyright the py.typed file should not be there. I also think that we should ignore mypy and focus on pyright which is a much better type checker. Also pyright is a more useful target because it is used by editors etc which are the most likely consumers of the hints. Presumably mypy has some options for this that you could use in CI. Personally I found that in code that used |
|
I'd like to see more specific log about how mypy --strict fails here, such that we can investigate if this can be fixed by changing your configuration or options. |
|
@sylee957 : When py.typed is removed, mypy simply refuses to scan the code! After spending such a long time adding mypy typehinting support to our codebase, it was quite jarring to see it removed. @oscarbenjamin : Wow, I really appreciate your efforts. After refactoring our codebase (NRPy, https://github.com/nrpy/nrpy) to support typehints, I understand what you mean by sympy not supporting typehints -- our task to add typehints to NRPy was far from trivial. At the time we had no idea of this policy! That said, even the very basic type hints currently inside sympy (for Expr, Basic, Symbol types) are potential lifesavers for large-ish codebases like NRPy, where being able to distinguish between Expr, List[Expr], etc., can mean the difference between functioning and mysteriously non-functioning code. May I please request that py.typed be restored? |
|
https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker You would likely need to do |
|
@sylee957 : Yes, I understand one could just force mypy to ignore sympy typehints. However, even though sympy's typehinting is very limited, even limited typehinting takes care of a large majority of our use cases. Bottom line: even a minimal amount of type hinting is valuable for improving code quality. |
|
I may be confusing what mypy is actually using from SymPy when analyzing our code. It could be that mypy only infers types from e.g., Expr, Symbol, Basic, etc. That said, removing |
|
I still think that there are other workaround for this
We would also note that SymPy's type annotation is very experimental, so unfortunately, we would better guide you to workaround for user breaking changes from here, than reverting this change. |
|
I haven't looked very hard but it does not look like there is an option to tell mypy to look at the sympy code when there is no py.typed file unless separate stub files are provided. With pyright it can be done with The fact that mypy works at all with the sympy codebase required many thousands of small changes only a few of which were fixes to actual bugs. It also only really "passes" because mypy is quite permissive about giving up on untyped code and mostly just fails to analyse the codebase. By contrast pyright which is stricter about wanting to actually infer the types either from hints or from static inference reports 40000 errors. It would be good to bring that number down and it does not really require 40000 type hints but rather a few well chosen ones for the most commonly used types and methods. For now the only reason I want to keep mypy running in CI is to stop people from adding incorrect type hints (which is what was happening before). I don't think that py.typed should be added back until there are some meaningful type hints in the sympy codebase. Passing basic checks under pyright seems like a reasonable baseline for this. |
|
Thank you for your thoughtful reply, @oscarbenjamin. To be honest, I don't think we're using many, if any, type hints implemented in sympy. Rather, we make use of sympy types, like converts all the sympy types to I wonder if the original issue that led to the removal of py.typed could be circumvented using the above trick. Regarding pyright, in my experience, it seems to be slower than mypy, even after I've cleared my mypy cache... and mypy is slow enough. 😛 @sylee957 Sadly, there are no typing stub packages for sympy. Truly |
Type hints are needed for any type checker to understand these. If you look at gh-25103 you can see that its main purpose is essentially to add hints for
It is unfortunate that mypy has this status because pyright is significantly better. Also the checker in pyright underpins pylance which is what is used in e.g. vscode. I am pretty sure that any type hints in SymPy would get far more use in editors like vscode than in downstream projects running mypy. I think that the primary reason that pyright is slower than mypy is because it is actually checking the types properly rather than using strange heuristic rules as mypy does. I am not sure that it would be possible to get the sympy codebase to the point that both pyright and mypy would be happy with it simultaneously because sympy does many strange things and the two checkers often disagree about nontrivial situations. Note also that when they do disagree pyright's opinion is pretty much always the correct one.
It does not prevent people from using mypy. It just means that mypy will have limited ability to make inference about any use of sympy. That is already the case anyway though: using mypy with sympy gives only a false notion of checking because mypy cannot understand the vast majority of sympy even if you restrict to "simple" types like Symbol. At least pyright has the decency to crash to show you that it cannot sensibly analyse this codebase! Also it would be better if mypy had the option to ask it to look at the sympy codebase as pyright does. That is a reasonable feature request for mypy. The default position for now though should be that type checkers do not look at the sympy codebase because it does not have the type hints that would be needed for them to perform sensibly. I would like to get to the point where sympy has enough type hints to be usable with type checkers but what is needed is the type hints. The py.typed file should only be added after there is reasonable coverage in terms of hints at least on commonly used public API. |
|
I think that we may need to discuss about contributing to ‘typeshed’ for example. However, basic type stub for modules, for example, can still be robust and helpful for users who want to navigate through sympy codebase I think that typeshed can be one place, however, I’m not sure if it is really like ‘definitelytyped’ for typescript, because it seems like having few 3rd party type definitions, and poor contribution guidelines about how to handle any non-maintainer of the typeshed project, but maintainers of SymPy, can edit the types, for instance. |
|
I still think that you can generate stubs, with mypy stubgen or pyright generate stubs, and try to maintain, or ship as 3rd party type stubs of sympy. |
|
But may not suffer from the existing performance of pyright for instance, trying to scan the whole library code by itself. |
|
The pyright folks have a suggestion to add type stub files for sympy: microsoft/pyright#7159 (comment) I'm not sure whether they are suggesting adding that to pyright or that we should add it to sympy. |
1 similar comment
|
The pyright folks have a suggestion to add type stub files for sympy: microsoft/pyright#7159 (comment) I'm not sure whether they are suggesting adding that to pyright or that we should add it to sympy. |
References to other Issues or PRs
Fixes #26150
Brief description of what is fixed or changed
We had discussed about removing
py.typedbecause it is not actually necessary for type checking,and it degrades performance for type checking for libraries with very few type coverage.
Although it can also disable some autocompletion features, which could be just working,
however, I wouldn't find it to give any useful information for users as well if the type coverage is poor.
However, because we are not frequently releasing, if users may need immediate changes, this may need hotfix for past releases as well.
I also want to check how users of mypy were not raising issues about performance anyway.
(Because I thought pyright was faster than mypy https://github.com/microsoft/pyright/blob/main/docs/mypy-comparison.md#design-goals)
Other comments
Release Notes
NO ENTRY