[blivet][master] Fix issues reported by pyflakes

[Vratislav Podzimek]

Unused imports and variables.

Signed-off-by: Vratislav Podzimek 
---
 blivet/__init__.py                  | 9 ---------
 blivet/formats/__init__.py          | 4 ++--
 tests/devicelibs_test/btrfs_test.py | 3 ---
 tests/partitioning_test.py          | 8 ++++----
 4 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/blivet/__init__.py b/blivet/__init__.py
index f2240ce..e384551 100644
--- a/blivet/__init__.py
+++ b/blivet/__init__.py
@@ -50,11 +50,8 @@ get_bootloader = lambda: None
 ##

import os
-from os import statvfs
 import time
 import stat
-import errno
-import sys
 import copy
 import tempfile
 import shlex
@@ -82,7 +79,6 @@ from .devicelibs.dm import name_from_dm_node
 from .devicelibs.crypto import generateBackupPassphrase
 from .devicelibs.edd import get_edd_dict
 from .devicelibs.dasd import make_dasd_list, write_dasd_conf
-from . import udev
 from . import iscsi
 from . import fcoe
 from . import zfcp
@@ -112,11 +108,6 @@ def enable_installer_mode():
     global ERROR_RAISE

from pyanaconda import iutil # pylint: disable=redefined-outer-name
-    from pyanaconda.constants import shortProductName # pylint: disable=redefined-outer-name
-    from pyanaconda.constants import productName # pylint: disable=redefined-outer-name
-    from pyanaconda.bootloader import get_bootloader # pylint: disable=redefined-outer-name
-    from pyanaconda.errors import errorHandler # pylint: disable=redefined-outer-name
-    from pyanaconda.errors import ERROR_RAISE # pylint: disable=redefined-outer-name

if hasattr(iutil, 'getTargetPhysicalRoot'):
         # For anaconda versions > 21.43
diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py
index eabd22f..70280e8 100644
--- a/blivet/formats/__init__.py
+++ b/blivet/formats/__init__.py
@@ -27,10 +27,10 @@ from ..util import get_sysfs_path_by_name
 from ..util import run_program
 from ..util import ObjectID
 from ..storage_log import log_method_call
-from ..errors import DeviceFormatError, DMError, FormatCreateError, FormatDestroyError, FormatSetupError, MDRaidError, StorageError
+from ..errors import DeviceFormatError, DMError, FormatCreateError, FormatDestroyError, FormatSetupError, MDRaidError
 from ..devicelibs.dm import dm_node_from_name
 from ..devicelibs.mdraid import md_node_from_name
-from ..i18n import _, N_
+from ..i18n import N_
 from ..size import Size

import logging
diff --git a/tests/devicelibs_test/btrfs_test.py b/tests/devicelibs_test/btrfs_test.py
index 42e2156..dcf1bcd 100755
--- a/tests/devicelibs_test/btrfs_test.py
+++ b/tests/devicelibs_test/btrfs_test.py
@@ -60,7 +60,6 @@ class BTRFSAsRootTestCase1(loopbackedtestcase.LoopBackedTestCase):
            These tests are limited to simple creating and scanning.
         """
         _LOOP_DEV0 = self.loopDevices[0]
-        _LOOP_DEV1 = self.loopDevices[1]

##
         ## create_volume
@@ -108,8 +107,6 @@ class BTRFSAsRootTestCase2(BTRFSMountDevice):

def testSubvolume(self):
         """Tests which focus on subvolumes."""
-        _LOOP_DEV0 = self.loopDevices[0]
-        _LOOP_DEV1 = self.loopDevices[1]

# no subvolumes yet
         self.assertEqual(btrfs.list_subvolumes(self.mountpoint), [])
diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
index 6b0d9b6..41ca1a0 100644
--- a/tests/partitioning_test.py
+++ b/tests/partitioning_test.py
@@ -194,10 +194,10 @@ class PartitioningTestCase(unittest.TestCase):
             placeholder = addPartition(disk.format, free,
                                        parted.PARTITION_NORMAL, Size("10 MiB"))
             all_free = disk.format.partedDisk.getFreeSpaceRegions()
-            extended = addPartition(disk.format, all_free[1],
-                                    parted.PARTITION_EXTENDED, Size("30 MiB"),
-                                    alignment.alignUp(all_free[1],
-                                                      placeholder.geometry.end))
+            addPartition(disk.format, all_free[1],
+                         parted.PARTITION_EXTENDED, Size("30 MiB"),
+                         alignment.alignUp(all_free[1],
+                                           placeholder.geometry.end))

disk.format.removePartition(placeholder)
             self.assertEqual(len(disk.format.partitions), 1)

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: [e-mail]
> Sent: Wednesday, July 30, 2014 10:17:37 AM
> Subject: [blivet][master] Fix issues reported by pyflakes
>
> Unused imports and variables.
> Signed-off-by: Vratislav Podzimek
>
> blivet/__init__.py                  | 9 ---------
>  blivet/formats/__init__.py          | 4 ++--
>  tests/devicelibs_test/btrfs_test.py | 3 ---
>  tests/partitioning_test.py          | 8 ++++----
>  4 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/blivet/__init__.py b/blivet/__init__.py
> index f2240ce..e384551 100644
> --- a/blivet/__init__.py
> +++ b/blivet/__init__.py
> @@ -50,11 +50,8 @@ get_bootloader = lambda: None
>  ##
>
> import os
> -from os import statvfs
>  import time
>  import stat
> -import errno
> -import sys
>  import copy
>  import tempfile
>  import shlex
> @@ -82,7 +79,6 @@ from .devicelibs.dm import name_from_dm_node
>  from .devicelibs.crypto import generateBackupPassphrase
>  from .devicelibs.edd import get_edd_dict
>  from .devicelibs.dasd import make_dasd_list, write_dasd_conf
> -from . import udev
>  from . import iscsi
>  from . import fcoe
>  from . import zfcp
> @@ -112,11 +108,6 @@ def enable_installer_mode():
>      global ERROR_RAISE
>  from pyanaconda import iutil # pylint: disable=redefined-outer-name
> from pyanaconda.constants import shortProductName # pylint:
>
> disable=redefined-outer-name
> from pyanaconda.constants import productName # pylint:
>
> disable=redefined-outer-name
> from pyanaconda.bootloader import get_bootloader # pylint:
>
> disable=redefined-outer-name
> from pyanaconda.errors import errorHandler # pylint:
>
> disable=redefined-outer-name
> from pyanaconda.errors import ERROR_RAISE # pylint:
>
> disable=redefined-outer-name

I think pyflakes is wrong about the above, though.
>  if hasattr(iutil, 'getTargetPhysicalRoot'):
>      # For anaconda versions > 21.43
>
> diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py
> index eabd22f..70280e8 100644
> --- a/blivet/formats/__init__.py
> +++ b/blivet/formats/__init__.py
> @@ -27,10 +27,10 @@ from ..util import get_sysfs_path_by_name
>  from ..util import run_program
>  from ..util import ObjectID
>  from ..storage_log import log_method_call
> -from ..errors import DeviceFormatError, DMError, FormatCreateError,
> FormatDestroyError, FormatSetupError, MDRaidError, StorageError
> +from ..errors import DeviceFormatError, DMError, FormatCreateError,
> FormatDestroyError, FormatSetupError, MDRaidError
>  from ..devicelibs.dm import dm_node_from_name
>  from ..devicelibs.mdraid import md_node_from_name
> -from ..i18n import _, N_
> +from ..i18n import N_
>  from ..size import Size
>
> import logging
> diff --git a/tests/devicelibs_test/btrfs_test.py
> b/tests/devicelibs_test/btrfs_test.py
> index 42e2156..dcf1bcd 100755
> --- a/tests/devicelibs_test/btrfs_test.py
> +++ b/tests/devicelibs_test/btrfs_test.py
> @@ -60,7 +60,6 @@ class
> BTRFSAsRootTestCase1(loopbackedtestcase.LoopBackedTestCase):
>             These tests are limited to simple creating and scanning.
>          """
>          _LOOP_DEV0 = self.loopDevices[0]
>    _LOOP_DEV1 = self.loopDevices[1]
>
>    ##
>    ## create_volume
>
> @@ -108,8 +107,6 @@ class BTRFSAsRootTestCase2(BTRFSMountDevice):
>  def testSubvolume(self):
>      """Tests which focus on subvolumes."""
>    _LOOP_DEV0 = self.loopDevices[0]
>    _LOOP_DEV1 = self.loopDevices[1]

pylint does not detect the above, wierd.
>      # no subvolumes yet
>      self.assertEqual(btrfs.list_subvolumes(self.mountpoint), [])
>
> diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
> index 6b0d9b6..41ca1a0 100644
> --- a/tests/partitioning_test.py
> +++ b/tests/partitioning_test.py
> @@ -194,10 +194,10 @@ class PartitioningTestCase(unittest.TestCase):
>              placeholder = addPartition(disk.format, free,
>                                         parted.PARTITION_NORMAL, Size("10
>                                         MiB"))
>              all_free = disk.format.partedDisk.getFreeSpaceRegions()
>        extended = addPartition(disk.format, all_free[1],
>                                parted.PARTITION_EXTENDED, Size("30
>
> MiB"),
>                                alignment.alignUp(all_free[1],
>
> placeholder.geometry.end))
>        addPartition(disk.format, all_free[1],
>                     parted.PARTITION_EXTENDED, Size("30 MiB"),
>                     alignment.alignUp(all_free[1],
>                                       placeholder.geometry.end))

pylint detects the above as well.
>          disk.format.removePartition(placeholder)
>          self.assertEqual(len(disk.format.partitions), 1)
>
> --
> 1.9.3
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

I've marked in the text a spot where it looks like pyflakes is wrong.

I think pylint is frightened of reporting unused imports in __init__.py
files for some reason, but I don't follow the reasoning.

Since pyflakes can catch some things that pylint does not we
should consider making it a Makefile target if possible.

- mulhern

[Vratislav Podzimek]

On Wed, 2014-07-30 at 05:26 -0400, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" 
> > To: [e-mail]
> > Sent: Wednesday, July 30, 2014 10:17:37 AM
> > Subject: [blivet][master] Fix issues reported by pyflakes
> >
> > Unused imports and variables.
> > Signed-off-by: Vratislav Podzimek
> >
> > blivet/__init__.py                  | 9 ---------
> >  blivet/formats/__init__.py          | 4 ++--
> >  tests/devicelibs_test/btrfs_test.py | 3 ---
> >  tests/partitioning_test.py          | 8 ++++----
> >  4 files changed, 6 insertions(+), 18 deletions(-)
> >
> > diff --git a/blivet/__init__.py b/blivet/__init__.py
> > index f2240ce..e384551 100644
> > --- a/blivet/__init__.py
> > +++ b/blivet/__init__.py
> > @@ -50,11 +50,8 @@ get_bootloader = lambda: None
> >  ##
> >
> > import os
> > -from os import statvfs
> >  import time
> >  import stat
> > -import errno
> > -import sys
> >  import copy
> >  import tempfile
> >  import shlex
> > @@ -82,7 +79,6 @@ from .devicelibs.dm import name_from_dm_node
> >  from .devicelibs.crypto import generateBackupPassphrase
> >  from .devicelibs.edd import get_edd_dict
> >  from .devicelibs.dasd import make_dasd_list, write_dasd_conf
> > -from . import udev
> >  from . import iscsi
> >  from . import fcoe
> >  from . import zfcp
> > @@ -112,11 +108,6 @@ def enable_installer_mode():
> >      global ERROR_RAISE
> >  from pyanaconda import iutil # pylint: disable=redefined-outer-name
> > from pyanaconda.constants import shortProductName # pylint:
> >
> > disable=redefined-outer-name
> > from pyanaconda.constants import productName # pylint:
> >
> > disable=redefined-outer-name
> > from pyanaconda.bootloader import get_bootloader # pylint:
> >
> > disable=redefined-outer-name
> > from pyanaconda.errors import errorHandler # pylint:
> >
> > disable=redefined-outer-name
> > from pyanaconda.errors import ERROR_RAISE # pylint:
> >
> > disable=redefined-outer-name
>
> I think pyflakes is wrong about the above, though.
> >  if hasattr(iutil, 'getTargetPhysicalRoot'):
> >      # For anaconda versions > 21.43
> >
> > diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py
> > index eabd22f..70280e8 100644
> > --- a/blivet/formats/__init__.py
> > +++ b/blivet/formats/__init__.py
> > @@ -27,10 +27,10 @@ from ..util import get_sysfs_path_by_name
> >  from ..util import run_program
> >  from ..util import ObjectID
> >  from ..storage_log import log_method_call
> > -from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > FormatDestroyError, FormatSetupError, MDRaidError, StorageError
> > +from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > FormatDestroyError, FormatSetupError, MDRaidError
> >  from ..devicelibs.dm import dm_node_from_name
> >  from ..devicelibs.mdraid import md_node_from_name
> > -from ..i18n import _, N_
> > +from ..i18n import N_
> >  from ..size import Size
> >
> > import logging
> > diff --git a/tests/devicelibs_test/btrfs_test.py
> > b/tests/devicelibs_test/btrfs_test.py
> > index 42e2156..dcf1bcd 100755
> > --- a/tests/devicelibs_test/btrfs_test.py
> > +++ b/tests/devicelibs_test/btrfs_test.py
> > @@ -60,7 +60,6 @@ class
> > BTRFSAsRootTestCase1(loopbackedtestcase.LoopBackedTestCase):
> >             These tests are limited to simple creating and scanning.
> >          """
> >          _LOOP_DEV0 = self.loopDevices[0]
> >    _LOOP_DEV1 = self.loopDevices[1]
> >
> >    ##
> >    ## create_volume
> >
> > @@ -108,8 +107,6 @@ class BTRFSAsRootTestCase2(BTRFSMountDevice):
> >  def testSubvolume(self):
> >      """Tests which focus on subvolumes."""
> >    _LOOP_DEV0 = self.loopDevices[0]
> >    _LOOP_DEV1 = self.loopDevices[1]
>
> pylint does not detect the above, wierd.
> >      # no subvolumes yet
> >      self.assertEqual(btrfs.list_subvolumes(self.mountpoint), [])
> >
> > diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
> > index 6b0d9b6..41ca1a0 100644
> > --- a/tests/partitioning_test.py
> > +++ b/tests/partitioning_test.py
> > @@ -194,10 +194,10 @@ class PartitioningTestCase(unittest.TestCase):
> >              placeholder = addPartition(disk.format, free,
> >                                         parted.PARTITION_NORMAL, Size("10
> >                                         MiB"))
> >              all_free = disk.format.partedDisk.getFreeSpaceRegions()
> >        extended = addPartition(disk.format, all_free[1],
> >                                parted.PARTITION_EXTENDED, Size("30
> >
> > MiB"),
> >                                alignment.alignUp(all_free[1],
> >
> > placeholder.geometry.end))
> >        addPartition(disk.format, all_free[1],
> >                     parted.PARTITION_EXTENDED, Size("30 MiB"),
> >                     alignment.alignUp(all_free[1],
> >                                       placeholder.geometry.end))
>
> pylint detects the above as well.
> >          disk.format.removePartition(placeholder)
> >          self.assertEqual(len(disk.format.partitions), 1)
> >
> > --
> > 1.9.3
> >
> > anaconda-patches mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> I've marked in the text a spot where it looks like pyflakes is wrong.

I've tried looking for the usage of the reported unused
imports/variables and none of them seems to be used. And pylint doesn't
have any problem with these changes.
> I think pylint is frightened of reporting unused imports in __init__.py
> files for some reason, but I don't follow the reasoning.
>
> Since pyflakes can catch some things that pylint does not we
> should consider making it a Makefile target if possible.

I had to skip some of the false-positives so it wouldn't be a good
choice for forcing it to pass, but a Makefile target could be useful. Or
everybody could enable pyflakes in their Vim/Emacs flymake setup. ;)

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: "anaconda patch review" 
> Sent: Wednesday, July 30, 2014 11:32:00 AM
> Subject: Re: [blivet][master] Fix issues reported by pyflakes
>
> On Wed, 2014-07-30 at 05:26 -0400, Anne Mulhern wrote:
> > ----- Original Message -----
> > > From: "Vratislav Podzimek" 
> > > To: [e-mail]
> > > Sent: Wednesday, July 30, 2014 10:17:37 AM
> > > Subject: [blivet][master] Fix issues reported by pyflakes
> > >
> > > Unused imports and variables.
> > > Signed-off-by: Vratislav Podzimek
> > >
> > > blivet/__init__.py                  | 9 ---------
> > >  blivet/formats/__init__.py          | 4 ++--
> > >  tests/devicelibs_test/btrfs_test.py | 3 ---
> > >  tests/partitioning_test.py          | 8 ++++----
> > >  4 files changed, 6 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/blivet/__init__.py b/blivet/__init__.py
> > > index f2240ce..e384551 100644
> > > --- a/blivet/__init__.py
> > > +++ b/blivet/__init__.py
> > > @@ -50,11 +50,8 @@ get_bootloader = lambda: None
> > >  ##
> > >
> > > import os
> > > -from os import statvfs
> > >  import time
> > >  import stat
> > > -import errno
> > > -import sys
> > >  import copy
> > >  import tempfile
> > >  import shlex
> > > @@ -82,7 +79,6 @@ from .devicelibs.dm import name_from_dm_node
> > >  from .devicelibs.crypto import generateBackupPassphrase
> > >  from .devicelibs.edd import get_edd_dict
> > >  from .devicelibs.dasd import make_dasd_list, write_dasd_conf
> > > -from . import udev
> > >  from . import iscsi
> > >  from . import fcoe
> > >  from . import zfcp
> > > @@ -112,11 +108,6 @@ def enable_installer_mode():
> > >      global ERROR_RAISE
> > >  from pyanaconda import iutil # pylint: disable=redefined-outer-name
> > > from pyanaconda.constants import shortProductName # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.constants import productName # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.bootloader import get_bootloader # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.errors import errorHandler # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.errors import ERROR_RAISE # pylint:
> > >
> > > disable=redefined-outer-name
> >
> > I think pyflakes is wrong about the above, though.
> > >  if hasattr(iutil, 'getTargetPhysicalRoot'):
> > >      # For anaconda versions > 21.43
> > >
> > > diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py
> > > index eabd22f..70280e8 100644
> > > --- a/blivet/formats/__init__.py
> > > +++ b/blivet/formats/__init__.py
> > > @@ -27,10 +27,10 @@ from ..util import get_sysfs_path_by_name
> > >  from ..util import run_program
> > >  from ..util import ObjectID
> > >  from ..storage_log import log_method_call
> > > -from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > > FormatDestroyError, FormatSetupError, MDRaidError, StorageError
> > > +from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > > FormatDestroyError, FormatSetupError, MDRaidError
> > >  from ..devicelibs.dm import dm_node_from_name
> > >  from ..devicelibs.mdraid import md_node_from_name
> > > -from ..i18n import _, N_
> > > +from ..i18n import N_
> > >  from ..size import Size
> > >
> > > import logging
> > > diff --git a/tests/devicelibs_test/btrfs_test.py
> > > b/tests/devicelibs_test/btrfs_test.py
> > > index 42e2156..dcf1bcd 100755
> > > --- a/tests/devicelibs_test/btrfs_test.py
> > > +++ b/tests/devicelibs_test/btrfs_test.py
> > > @@ -60,7 +60,6 @@ class
> > > BTRFSAsRootTestCase1(loopbackedtestcase.LoopBackedTestCase):
> > >             These tests are limited to simple creating and scanning.
> > >          """
> > >          _LOOP_DEV0 = self.loopDevices[0]
> > >    _LOOP_DEV1 = self.loopDevices[1]
> > >
> > >    ##
> > >    ## create_volume
> > >
> > > @@ -108,8 +107,6 @@ class BTRFSAsRootTestCase2(BTRFSMountDevice):
> > >  def testSubvolume(self):
> > >      """Tests which focus on subvolumes."""
> > >    _LOOP_DEV0 = self.loopDevices[0]
> > >    _LOOP_DEV1 = self.loopDevices[1]
> >
> > pylint does not detect the above, wierd.
> > >      # no subvolumes yet
> > >      self.assertEqual(btrfs.list_subvolumes(self.mountpoint), [])
> > >
> > > diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
> > > index 6b0d9b6..41ca1a0 100644
> > > --- a/tests/partitioning_test.py
> > > +++ b/tests/partitioning_test.py
> > > @@ -194,10 +194,10 @@ class PartitioningTestCase(unittest.TestCase):
> > >              placeholder = addPartition(disk.format, free,
> > >                                         parted.PARTITION_NORMAL, Size("10
> > >                                         MiB"))
> > >              all_free = disk.format.partedDisk.getFreeSpaceRegions()
> > >        extended = addPartition(disk.format, all_free[1],
> > >                                parted.PARTITION_EXTENDED, Size("30
> > >
> > > MiB"),
> > >                                alignment.alignUp(all_free[1],
> > >
> > > placeholder.geometry.end))
> > >        addPartition(disk.format, all_free[1],
> > >                     parted.PARTITION_EXTENDED, Size("30 MiB"),
> > >                     alignment.alignUp(all_free[1],
> > >                                       placeholder.geometry.end))
> >
> > pylint detects the above as well.
> > >          disk.format.removePartition(placeholder)
> > >          self.assertEqual(len(disk.format.partitions), 1)
> > >
> > > --
> > > 1.9.3
> > >
> > > anaconda-patches mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
> > I've marked in the text a spot where it looks like pyflakes is wrong.
>
> I've tried looking for the usage of the reported unused
> imports/variables and none of them seems to be used. And pylint doesn't
> have any problem with these changes.

Whatever's in your directory is a tad bit ahead of the current version of
master in the repo, then.

[mulhern@localhost blivet]$ grep shortProductName blivet/__init__.py
shortProductName = 'blivet'
    global shortProductName
    from pyanaconda.constants import shortProductName # pylint: disable=redefined-outer-name
            name = self.suggestDeviceName(prefix=shortProductName,
            prefix = shortProductName

There's no reason why pylint should complain if those are removed, in the
code base that I see.
> > I think pylint is frightened of reporting unused imports in __init__.py
> > files for some reason, but I don't follow the reasoning.
> >
> > Since pyflakes can catch some things that pylint does not we
> > should consider making it a Makefile target if possible.
>
> I had to skip some of the false-positives so it wouldn't be a good
> choice for forcing it to pass, but a Makefile target could be useful. Or
> everybody could enable pyflakes in their Vim/Emacs flymake setup. ;)

We skip a few false positives in pylint, too, so there might still be a benefit.
I'll put it on my list of wishes.

- mulhern
> --
> Vratislav Podzimek
>
> Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

[Vratislav Podzimek]

On Wed, 2014-07-30 at 05:49 -0400, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" 
> > To: "anaconda patch review" 
> > Sent: Wednesday, July 30, 2014 11:32:00 AM
> > Subject: Re: [blivet][master] Fix issues reported by pyflakes
> >
> > On Wed, 2014-07-30 at 05:26 -0400, Anne Mulhern wrote:
> > > ----- Original Message -----
> > > > From: "Vratislav Podzimek" 
> > > > To: [e-mail]
> > > > Sent: Wednesday, July 30, 2014 10:17:37 AM
> > > > Subject: [blivet][master] Fix issues reported by pyflakes
> > > >
> > > > Unused imports and variables.
> > > > Signed-off-by: Vratislav Podzimek
> > > >
> > > > blivet/__init__.py                  | 9 ---------
> > > >  blivet/formats/__init__.py          | 4 ++--
> > > >  tests/devicelibs_test/btrfs_test.py | 3 ---
> > > >  tests/partitioning_test.py          | 8 ++++----
> > > >  4 files changed, 6 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/blivet/__init__.py b/blivet/__init__.py
> > > > index f2240ce..e384551 100644
> > > > --- a/blivet/__init__.py
> > > > +++ b/blivet/__init__.py
> > > > @@ -50,11 +50,8 @@ get_bootloader = lambda: None
> > > >  ##
> > > >
> > > > import os
> > > > -from os import statvfs
> > > >  import time
> > > >  import stat
> > > > -import errno
> > > > -import sys
> > > >  import copy
> > > >  import tempfile
> > > >  import shlex
> > > > @@ -82,7 +79,6 @@ from .devicelibs.dm import name_from_dm_node
> > > >  from .devicelibs.crypto import generateBackupPassphrase
> > > >  from .devicelibs.edd import get_edd_dict
> > > >  from .devicelibs.dasd import make_dasd_list, write_dasd_conf
> > > > -from . import udev
> > > >  from . import iscsi
> > > >  from . import fcoe
> > > >  from . import zfcp
> > > > @@ -112,11 +108,6 @@ def enable_installer_mode():
> > > >      global ERROR_RAISE
> > > >  from pyanaconda import iutil # pylint: disable=redefined-outer-name
> > > > from pyanaconda.constants import shortProductName # pylint:
> > > >
> > > > disable=redefined-outer-name
> > > > from pyanaconda.constants import productName # pylint:
> > > >
> > > > disable=redefined-outer-name
> > > > from pyanaconda.bootloader import get_bootloader # pylint:
> > > >
> > > > disable=redefined-outer-name
> > > > from pyanaconda.errors import errorHandler # pylint:
> > > >
> > > > disable=redefined-outer-name
> > > > from pyanaconda.errors import ERROR_RAISE # pylint:
> > > >
> > > > disable=redefined-outer-name
> > >
> > > I think pyflakes is wrong about the above, though.
> > > >  if hasattr(iutil, 'getTargetPhysicalRoot'):
> > > >      # For anaconda versions > 21.43
> > > >
> > > > diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py
> > > > index eabd22f..70280e8 100644
> > > > --- a/blivet/formats/__init__.py
> > > > +++ b/blivet/formats/__init__.py
> > > > @@ -27,10 +27,10 @@ from ..util import get_sysfs_path_by_name
> > > >  from ..util import run_program
> > > >  from ..util import ObjectID
> > > >  from ..storage_log import log_method_call
> > > > -from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > > > FormatDestroyError, FormatSetupError, MDRaidError, StorageError
> > > > +from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > > > FormatDestroyError, FormatSetupError, MDRaidError
> > > >  from ..devicelibs.dm import dm_node_from_name
> > > >  from ..devicelibs.mdraid import md_node_from_name
> > > > -from ..i18n import _, N_
> > > > +from ..i18n import N_
> > > >  from ..size import Size
> > > >
> > > > import logging
> > > > diff --git a/tests/devicelibs_test/btrfs_test.py
> > > > b/tests/devicelibs_test/btrfs_test.py
> > > > index 42e2156..dcf1bcd 100755
> > > > --- a/tests/devicelibs_test/btrfs_test.py
> > > > +++ b/tests/devicelibs_test/btrfs_test.py
> > > > @@ -60,7 +60,6 @@ class
> > > > BTRFSAsRootTestCase1(loopbackedtestcase.LoopBackedTestCase):
> > > >             These tests are limited to simple creating and scanning.
> > > >          """
> > > >          _LOOP_DEV0 = self.loopDevices[0]
> > > >    _LOOP_DEV1 = self.loopDevices[1]
> > > >
> > > >    ##
> > > >    ## create_volume
> > > >
> > > > @@ -108,8 +107,6 @@ class BTRFSAsRootTestCase2(BTRFSMountDevice):
> > > >  def testSubvolume(self):
> > > >      """Tests which focus on subvolumes."""
> > > >    _LOOP_DEV0 = self.loopDevices[0]
> > > >    _LOOP_DEV1 = self.loopDevices[1]
> > >
> > > pylint does not detect the above, wierd.
> > > >      # no subvolumes yet
> > > >      self.assertEqual(btrfs.list_subvolumes(self.mountpoint), [])
> > > >
> > > > diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
> > > > index 6b0d9b6..41ca1a0 100644
> > > > --- a/tests/partitioning_test.py
> > > > +++ b/tests/partitioning_test.py
> > > > @@ -194,10 +194,10 @@ class PartitioningTestCase(unittest.TestCase):
> > > >              placeholder = addPartition(disk.format, free,
> > > >                                         parted.PARTITION_NORMAL, Size("10
> > > >                                         MiB"))
> > > >              all_free = disk.format.partedDisk.getFreeSpaceRegions()
> > > >        extended = addPartition(disk.format, all_free[1],
> > > >                                parted.PARTITION_EXTENDED, Size("30
> > > >
> > > > MiB"),
> > > >                                alignment.alignUp(all_free[1],
> > > >
> > > > placeholder.geometry.end))
> > > >        addPartition(disk.format, all_free[1],
> > > >                     parted.PARTITION_EXTENDED, Size("30 MiB"),
> > > >                     alignment.alignUp(all_free[1],
> > > >                                       placeholder.geometry.end))
> > >
> > > pylint detects the above as well.
> > > >          disk.format.removePartition(placeholder)
> > > >          self.assertEqual(len(disk.format.partitions), 1)
> > > >
> > > > --
> > > > 1.9.3
> > > >
> > > > anaconda-patches mailing list
> > > > [e-mail]
> > > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > >
> > > I've marked in the text a spot where it looks like pyflakes is wrong.
> >
> > I've tried looking for the usage of the reported unused
> > imports/variables and none of them seems to be used. And pylint doesn't
> > have any problem with these changes.
>
> Whatever's in your directory is a tad bit ahead of the current version of
> master in the repo, then.
>
> [mulhern@localhost blivet]$ grep shortProductName blivet/__init__.py
> shortProductName = 'blivet'
>     global shortProductName
>     from pyanaconda.constants import shortProductName # pylint: disable=redefined-outer-name
>             name = self.suggestDeviceName(prefix=shortProductName,
>             prefix = shortProductName
>
> There's no reason why pylint should complain if those are removed, in the
> code base that I see.

Yeah, you're right. It's just productName that is no longer used with
the sanityCheck moved. Thanks for double-checking this! I'll fix it
locally and keep those pyanaconda.* imports in place. pyflakes gets
confused by the way we hide global variables with those imports, I
guess.

[Martin Kolman]

On Wed, 2014-07-30 at 11:32 +0200, Vratislav Podzimek wrote:
> On Wed, 2014-07-30 at 05:26 -0400, Anne Mulhern wrote:
> > ----- Original Message -----
> > > From: "Vratislav Podzimek" 
> > > To: [e-mail]
> > > Sent: Wednesday, July 30, 2014 10:17:37 AM
> > > Subject: [blivet][master] Fix issues reported by pyflakes
> > >
> > > Unused imports and variables.
> > > Signed-off-by: Vratislav Podzimek
> > >
> > > blivet/__init__.py                  | 9 ---------
> > >  blivet/formats/__init__.py          | 4 ++--
> > >  tests/devicelibs_test/btrfs_test.py | 3 ---
> > >  tests/partitioning_test.py          | 8 ++++----
> > >  4 files changed, 6 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/blivet/__init__.py b/blivet/__init__.py
> > > index f2240ce..e384551 100644
> > > --- a/blivet/__init__.py
> > > +++ b/blivet/__init__.py
> > > @@ -50,11 +50,8 @@ get_bootloader = lambda: None
> > >  ##
> > >
> > > import os
> > > -from os import statvfs
> > >  import time
> > >  import stat
> > > -import errno
> > > -import sys
> > >  import copy
> > >  import tempfile
> > >  import shlex
> > > @@ -82,7 +79,6 @@ from .devicelibs.dm import name_from_dm_node
> > >  from .devicelibs.crypto import generateBackupPassphrase
> > >  from .devicelibs.edd import get_edd_dict
> > >  from .devicelibs.dasd import make_dasd_list, write_dasd_conf
> > > -from . import udev
> > >  from . import iscsi
> > >  from . import fcoe
> > >  from . import zfcp
> > > @@ -112,11 +108,6 @@ def enable_installer_mode():
> > >      global ERROR_RAISE
> > >  from pyanaconda import iutil # pylint: disable=redefined-outer-name
> > > from pyanaconda.constants import shortProductName # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.constants import productName # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.bootloader import get_bootloader # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.errors import errorHandler # pylint:
> > >
> > > disable=redefined-outer-name
> > > from pyanaconda.errors import ERROR_RAISE # pylint:
> > >
> > > disable=redefined-outer-name
> >
> > I think pyflakes is wrong about the above, though.
> > >  if hasattr(iutil, 'getTargetPhysicalRoot'):
> > >      # For anaconda versions > 21.43
> > >
> > > diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py
> > > index eabd22f..70280e8 100644
> > > --- a/blivet/formats/__init__.py
> > > +++ b/blivet/formats/__init__.py
> > > @@ -27,10 +27,10 @@ from ..util import get_sysfs_path_by_name
> > >  from ..util import run_program
> > >  from ..util import ObjectID
> > >  from ..storage_log import log_method_call
> > > -from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > > FormatDestroyError, FormatSetupError, MDRaidError, StorageError
> > > +from ..errors import DeviceFormatError, DMError, FormatCreateError,
> > > FormatDestroyError, FormatSetupError, MDRaidError
> > >  from ..devicelibs.dm import dm_node_from_name
> > >  from ..devicelibs.mdraid import md_node_from_name
> > > -from ..i18n import _, N_
> > > +from ..i18n import N_
> > >  from ..size import Size
> > >
> > > import logging
> > > diff --git a/tests/devicelibs_test/btrfs_test.py
> > > b/tests/devicelibs_test/btrfs_test.py
> > > index 42e2156..dcf1bcd 100755
> > > --- a/tests/devicelibs_test/btrfs_test.py
> > > +++ b/tests/devicelibs_test/btrfs_test.py
> > > @@ -60,7 +60,6 @@ class
> > > BTRFSAsRootTestCase1(loopbackedtestcase.LoopBackedTestCase):
> > >             These tests are limited to simple creating and scanning.
> > >          """
> > >          _LOOP_DEV0 = self.loopDevices[0]
> > >    _LOOP_DEV1 = self.loopDevices[1]
> > >
> > >    ##
> > >    ## create_volume
> > >
> > > @@ -108,8 +107,6 @@ class BTRFSAsRootTestCase2(BTRFSMountDevice):
> > >  def testSubvolume(self):
> > >      """Tests which focus on subvolumes."""
> > >    _LOOP_DEV0 = self.loopDevices[0]
> > >    _LOOP_DEV1 = self.loopDevices[1]
> >
> > pylint does not detect the above, wierd.
> > >      # no subvolumes yet
> > >      self.assertEqual(btrfs.list_subvolumes(self.mountpoint), [])
> > >
> > > diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
> > > index 6b0d9b6..41ca1a0 100644
> > > --- a/tests/partitioning_test.py
> > > +++ b/tests/partitioning_test.py
> > > @@ -194,10 +194,10 @@ class PartitioningTestCase(unittest.TestCase):
> > >              placeholder = addPartition(disk.format, free,
> > >                                         parted.PARTITION_NORMAL, Size("10
> > >                                         MiB"))
> > >              all_free = disk.format.partedDisk.getFreeSpaceRegions()
> > >        extended = addPartition(disk.format, all_free[1],
> > >                                parted.PARTITION_EXTENDED, Size("30
> > >
> > > MiB"),
> > >                                alignment.alignUp(all_free[1],
> > >
> > > placeholder.geometry.end))
> > >        addPartition(disk.format, all_free[1],
> > >                     parted.PARTITION_EXTENDED, Size("30 MiB"),
> > >                     alignment.alignUp(all_free[1],
> > >                                       placeholder.geometry.end))
> >
> > pylint detects the above as well.
> > >          disk.format.removePartition(placeholder)
> > >          self.assertEqual(len(disk.format.partitions), 1)
> > >
> > > --
> > > 1.9.3
> > >
> > > anaconda-patches mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
> > I've marked in the text a spot where it looks like pyflakes is wrong.
>
> I've tried looking for the usage of the reported unused
> imports/variables and none of them seems to be used. And pylint doesn't
> have any problem with these changes.
> > I think pylint is frightened of reporting unused imports in __init__.py
> > files for some reason, but I don't follow the reasoning.
> >
> > Since pyflakes can catch some things that pylint does not we
> > should consider making it a Makefile target if possible.
>
> I had to skip some of the false-positives so it wouldn't be a good
> choice for forcing it to pass, but a Makefile target could be useful. Or
> everybody could enable pyflakes in their Vim/Emacs flymake setup. ;)

I'm already using Pyflakes & co in my Vim setup and it works nicely, can
recommend. A bit distracting on RHEL7 & RHEL6 (warning/error overload)
code though. :)

[Brian C. Lane]

On Wed, Jul 30, 2014 at 05:26:39AM -0400, Anne Mulhern wrote:
> I think pylint is frightened of reporting unused imports in __init__.py
> files for some reason, but I don't follow the reasoning.

Probably because some people use __init__.py to import parts of the module
to make the namespace cleaner.