-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[PyQGIS] Add message to /Deprecated/ annotations #59834
Conversation
@m-kuhn the following method has been deprecated only in SIP not in C++ code : Reading the PR comments, I fail to see if it's still relevant, do you know if it still is? I planned in this PR to completely forbid this behavior (deprecated in Python, not in C++) and this is the only one failing at the moment. Do I have to reconsider and allow this kind of behavior? Or make an exception for this one? |
I think this should be permitted, eg for cases where we accidentally exposed something we'd rather keep C++ only. (But if it's too much trouble to support we could resort to marking the C++ one deprecated and then |
Regarding
Couldn't we use some regex magic in cmake when processing the .sip.in files to remove / retain these based on a cmake sip version variable? (Or manually set Boolean variable) |
🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. 🪟 Windows buildsDownload Windows builds of this PR for testing. |
I thought about that but the cmake configure() allows only to replace variables. But maybe we could create our own cmake command that replace variables and strip Deprecated message when the SIP version is lower than 6.9.0. I'm wondering if it isn't a bit ugly and fragile. |
Maybe add a SIP_PYTHON_ONLY_DEPRECATED to make the difference with all other ones where we want to check that Q_DECL_DEPRECATED, SIP_DEPRECATED and \deprecated are consistent |
@troopa81 I had a very quick read and I might be wrong, but the conditional directive depending on the SIP version looks something I've done in the past. That's why we have .in files, so they can be configured depending on the SIP version. Again, I am not sure this helps, but it's worth a try. |
0f0fd79
to
a67d90d
Compare
@nyalldawson and @3nids I fixed the sip version issue with some cmake magic, /deprecated/ is now pushed with message and this message is removed with cmake when sip is less than 6.9.0 I also removed the string requirement on \deprecated when SIP_DEPRECATED is used so QgsGeometry::set can stay the way it is |
LGTM, thanks! |
Works only with SIP 6.9.0
Use cmake to remove text when SIP version is less than 6.9.0
Because some method (only QgsGeometry::set actually) can be deprecated only in python, not in C++. But \deprecated triggers the need for Q_DECL_DEPRECATED
1ccf058
to
4eab531
Compare
This PR implements the QGIS part of QEP #287
It follows Python-SIP pull request #46 which allows to generate @deprecated decorator instruction in pyi files and support deprecated annotation with message. This new feature is only available in SIP 6.9.0.
As a result, if a python plugin use the deprecated function
QgsProject.fileInfo()
for instance,When running, you'd get the following (same as before but with the \deprecated message)
With using static Python linter pyright, you'd get the following
pyright configuration file pyproject.toml
It's not possible to activate the deprecated message only for version greater or equal than SIP 6.9.0 using %if directive (with either %Feature or SIP range version) because of this issue fixed now (but would fail with all versions below 6.9.0).
So the only remaining option was to have a separate folder like we did for Qt6. This would lead to even more sip files, so I prefer not to. One solution would be, maybe, to generate those files at packaging time, before building.
This PR proposes also to: