-
Notifications
You must be signed in to change notification settings - Fork 205
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
Display error dialog if python3-suds is not installed when querying RR #1050
Conversation
9bb544c
to
724bfaa
Compare
@@ -1835,7 +1836,12 @@ def _do_network_query(self, query_cls): | |||
self.add_editorset(editorset) | |||
|
|||
def _menu_query_rr(self, event): | |||
self._do_network_query(query_sources.RRQueryDialog) | |||
if radioreference.HAVE_SUDS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this (optionality and flag) is legacy and not actually because of linux, but macOS. Long ago, the frozen-in-time macos build runtime was missing the suds library and there wasn't an easy way to fix it. As of the new UI/python3 rewrite, that's no longer the case. Because of this optionality, it was also (mistakenly) left out of the build requirements, but not intentionally.
https://github.com/kk7ds/chirp/blob/master/setup.py#L15
Point is, it is now required, we should assume it's present. I'd rather we just remove all this optional import stuff and treat it like anything else.
self._do_network_query(query_sources.RRQueryDialog) | ||
else: | ||
common.error_proof.show_error( | ||
_('The python3-suds package is required ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps you inferred that Linux was the problem child because this package isn't listed in the set of distro packages on the linux page. However, linux users are getting that via pip/pipx currently.
Especially with the "managed environment" flag in modern distros, we might as well just install everything we can from pip instead of depending on the problems that come from varying distro version support for things (wxpython being the exception that we really need to get from the distro).
I was kind of following the instructions from the INSTALL file:
This file is not up-to-date anymore, but if you try following it and don't install python-suds (python3-suds actually), you don't get any feedback if you try to query RadioReference. Just an error on the console. I think Debian also picked up on this somehow, as they list python3-suds as a recommended dependency rather than a straight dependency (see https://packages.debian.org/trixie/chirp). Though it doesn't actually matter, because python3-suds still gets installed automatically with the chirp package. But overall, I prefer your suggestion. I will close this MR and open another one stripping the optional dependency instead. |
Ah, yep, those instructions are ancient, obviously. TBH, as much as python is a moving target these days, I'd prefer not to have to maintain a stale-ish set of instructions in the tree itself and point people to the more-kept-updated wiki page. Perhaps we should just replace it with some basic details and a "see more" link :) |
Speaking of installing CHIRP and modifying the INSTALL file, many distributions have packaged CHIRP-next: Debian: https://packages.debian.org/trixie/chirp What are your thoughts on providing installation instructions for distro packages in https://chirpmyradio.com/projects/chirp/wiki/ChirpOnLinux ? |
They're usually fairly old as the packages don't get updated very often. We'll almost always require people to reproduce a bug with the latest version anyway. So, we could mention that "your distro might have a chirp package" but I'd encourage people to run the latest instead. If we operated break-and-fix or maintained stable version branches (as we used to do) then it'd be different. |
On Linux, the python3-suds is an optional dependency. When it is not installed, it doesn't do anything in the GUI; it only displays an error on the console.