Repository: pyinstaller/pyinstaller
Pull request: utils/hooks : is_module_satisfies() now returns the correct result for packages not found on the system.
'' in string returns True in Python.
Previously return version in requirements_parsed returned True.
Now added an if statement to check for an empty string check for version,
before returning the value of the function.
Fixes #3428

Problem statement:
is_module_satisfies() returns True for packages that are not installed
`is_module_satisfies()` should return `False` if the package being provided is not installed.
Instead I get:

    >>> from PyInstaller.utils.hooks import is_module_satisfies
    >>> is_module_satisfies("xyz")
    Traceback (most recent call last):
    File "<string>", line 2, in <module>
    ImportError: No module named xyz
    True
    >>>

Hints from issue discussion:
This is cause by [this line](https://github.com/pyinstaller/pyinstaller/blob/v3.3.1/PyInstaller/utils/hooks/__init__.py#L492) setting version to an empty string if the module does not exist. That makes the test just below evaluate to true.

Needs test-case :-)
Hi @htgoebel can I get assigned to this issue? I think I fixed it on my branch. If you could tell me how I could start a PR it would be awesome sir. 
Feel free to provide a pull-request for this. We are not using "Assignment" in the project. (And github technically does not all allow to set you as "Assignee".) I'm looking forward to you contribution :-)


See [Creating pull requests](http://pyinstaller.readthedocs.io/en/v3.3.1/development/pull-requests.html?highlight=pull) and [commit messages](http://pyinstaller.readthedocs.io/en/stable/development/commit-messages.html?highlight=commit) for guidelines.
Thank you sir. There is also one thing I wanted to get clarification on. The exec_statement function creates a new shell and tries to import the package. This throws a ModuleNotFoundException if the package is not found, but the function returns the correct value anyways. Is that ok or what should the correct behaviour be?
I am attaching the code for that part here.
```python
def get_module_attribute(module_name, attr_name):
attr_value_if_undefined = '!)ABadCafe@(D15ea5e#*DeadBeef$&Fee1Dead%^'
    attr_value = exec_statement("""
        import %s as m
        print(getattr(m, %r, %r))
    """ % (module_name, attr_name, attr_value_if_undefined))
    if attr_value == attr_value_if_undefined:
        raise AttributeError(
            'Module %r has no attribute %r' % (module_name, attr_name))
    else:
        return attr_value
```
Some comments from the module were removed before pasting here. These are lines from 260-298 of the highlighted function.
```
>>> from PyInstaller.utils.hooks import is_module_satisfies
>>> a=is_module_satisfies('abd')
Traceback (most recent call last):
  File "<string>", line 2, in <module>
ModuleNotFoundError: No module named 'abd'
>>> a
False
```
This is the output after I made the change for the empty string check.

Commit message: Hooks: Added empty string check for version in is_module_satisfies()

'' in string returns True in Python.
Previously return version in requirements_parsed returned True.
Now added an if statement to check for an empty string check for version,
before returning the value of the function.
Fixes #3428

Patch to review:
diff --git a/PyInstaller/utils/hooks/__init__.py b/PyInstaller/utils/hooks/__init__.py
index 65e38b78812..b62ce0fedf3 100755
--- a/PyInstaller/utils/hooks/__init__.py
+++ b/PyInstaller/utils/hooks/__init__.py
@@ -492,7 +492,10 @@ class method (e.g., `idontevenknow<1.6,>1.9,!=1.9.6,<2.0a0,==2.4c1`) is
         version = get_module_attribute(module_name, version_attr)
 
     # Compare this version against the version parsed from these requirements.
-    return version in requirements_parsed
+    if version is '':
+        return False
+    else:
+        return version in requirements_parsed
 
 
 def is_package(module_name):


Code review comments:
diff_hunk: @@ -492,7 +492,10 @@ class method (e.g., `idontevenknow<1.6,>1.9,!=1.9.6,<2.0a0,==2.4c1`) is
         version = get_module_attribute(module_name, version_attr)
 
     # Compare this version against the version parsed from these requirements.
-    return version in requirements_parsed
+    if version is '':
path: PyInstaller/utils/hooks/__init__.py
text: @user1:
I suggest simply checking for `not version` which will match even if `get_module_attribute()` would return `None`.

Also this should go in front of the comment and get a comment of its own like "module does not exist".