Skip to content
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.

Sign up for GitHub

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

style: address flake8 e402 warning in the codebase #3354

Closed
wants to merge 6 commits into from
Closed

style: address flake8 e402 warning in the codebase #3354

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
40b60a4
style: address flake8 e402 warning in the codebase
Sharansrj567 Jan 11, 2024
c8ab468
Merge branch 'main' into fix-flask8-e402-warning
Sharansrj567 Jan 11, 2024
ea17773
refactor: bring modules at high level solving flake8 E402 warning
Sharansrj567 Jan 12, 2024
9a36b7e
Merge branch 'main' into fix-flask8-e402-warning
Sharansrj567 Jan 16, 2024
7030bd1
Merge remote-tracking branch 'origin/main' into pr/Sharansrj567/3354
echoix Dec 27, 2024
b55f997
Apply isort to changed files only when appropriate
echoix Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions gui/wxpython/core/gcmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,24 @@
"""

import os
import wx
import sys
import time
import errno
import signal
import traceback
import locale
import traceback
import subprocess

from threading import Thread
import wx
from core.debug import Debug
from core.globalvar import SCT_EXT

from grass.script import core as grass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if that's not from this PR, this doesn't sound right, and quite ambiguous (the alias's name)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed that across our codebase, there is a consistent use of the alias grass for the core module. Given that this pattern is present in around 51 files, it might be worth discussing whether we want to maintain this convention or consider a more descriptive alias.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this was introduced when transitioning form a single-file package called grass(.script?) to multiple files (modules). Functions in original grass or something like that were moved core and grass or grass.script got new functions. grass.script.core then became what grass once was. These imports were the least intrusive change, so it was implemented that way. This was in the pre-v7-times on Subversion trunk, but it was propagated sufficiently through the code base at the time of v7 release that it became a standard for v7. We came up with grass.script as gs as a better practice, but didn't go back (yet) to replace all occurrences of core as grass.

from grass.script.utils import decode, encode

is_mswindows = sys.platform == "win32"

if is_mswindows:
from win32file import ReadFile, WriteFile
from win32pipe import PeekNamedPipe
Expand All @@ -45,12 +52,6 @@
import select
import fcntl

from core.debug import Debug
from core.globalvar import SCT_EXT

from grass.script import core as grass
from grass.script.utils import decode, encode


def DecodeString(string):
"""Decode string using system encoding
Expand Down
8 changes: 3 additions & 5 deletions gui/wxpython/core/watchdog.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file I'm ok with.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

import os
import time
import wx
from wx.lib.newevent import NewEvent
from grass.script import core as grass

watchdog_used = True
try:
Expand All @@ -32,11 +35,6 @@
PatternMatchingEventHandler = object
FileSystemEventHandler = object

import wx
from wx.lib.newevent import NewEvent

from grass.script import core as grass

updateMapset, EVT_UPDATE_MAPSET = NewEvent()
currentMapsetChanged, EVT_CURRENT_MAPSET_CHANGED = NewEvent()

Expand Down
4 changes: 2 additions & 2 deletions gui/wxpython/datacatalog/tree.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file I'm ok with.

Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
"""
import os
import re
import wx
import copy
from multiprocessing import Process, Queue, cpu_count

import wx
import grass.script as gscript

from core.gcmd import RunCommand, GError, GMessage
from core.utils import GetListOfLocations
Expand Down Expand Up @@ -63,7 +64,6 @@

from grass.pydispatch.signal import Signal

import grass.script as gscript
from grass.script import gisenv
from grass.grassdb.data import map_exists
from grass.grassdb.checks import (
Expand Down
4 changes: 2 additions & 2 deletions gui/wxpython/docs/wxgui_sphinx/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import string
from shutil import copy

from grass.script import core

# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
# documentation root, use os.path.abspath to make it absolute, like shown here.
Expand All @@ -25,8 +27,6 @@
0, os.path.abspath(os.path.join(os.environ["GISBASE"], "etc", "python", "grass"))
)

from grass.script import core

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not behave successfully. Is there a way to know if it imports well, and not only wrapped inside a grass --exec python. I've hit some long import chains that needed everything already up and running in order to have something similar working. To confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran GRASS GIS, accessed the interactive terminal, entered the Python shell within that environment, and executed the import statement without encountering any errors.

Copy link
Member

@echoix echoix Feb 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I see that this file is for generating docs (sphinx). Does someone know how to trigger it to check it? I know that the docs are built daily on an OSGeo server

footer_tmpl = string.Template(
r"""
{% block footer %}<hr class="header">
Expand Down
5 changes: 2 additions & 3 deletions gui/wxpython/gmodeler/frame.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is most probably fine.

Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,11 @@
IsDark,
)
from gui_core.wrap import TextEntryDialog as wxTextEntryDialog

wxModelDone, EVT_MODEL_DONE = NewEvent()

from grass.script.utils import try_remove
from grass.script import core as grass

wxModelDone, EVT_MODEL_DONE = NewEvent()


class ModelFrame(wx.Frame):
def __init__(
Expand Down
3 changes: 1 addition & 2 deletions gui/wxpython/startup/locdownload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@
from grass.grassdb.checks import is_location_valid
from grass.script.setup import set_gui_path

set_gui_path()

from core.debug import Debug
from core.gthread import gThread
from gui_core.wrap import Button, StaticText

set_gui_path()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has there been a careful analysis of what is needed for gui_core.wrap import to be successful, and what does set_gui_path provide?


# TODO: labels (and descriptions) translatable?
LOCATIONS = [
Expand Down
26 changes: 10 additions & 16 deletions gui/wxpython/web_services/widgets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,23 @@

import re
import os
import wx
import sys
import shutil
import grass.script as grass
import wx.lib.colourselect as csel

from copy import deepcopy
from core import globalvar

from xml.etree.ElementTree import ParseError

import wx

if globalvar.wxPythonPhoenix:
try:
import agw.flatnotebook as FN
except ImportError: # if it's not there locally, try the wxPython lib.
import wx.lib.agw.flatnotebook as FN
else:
import wx.lib.flatnotebook as FN
import wx.lib.colourselect as csel

from core.debug import Debug
from core.gcmd import GMessage
from core.gconsole import CmdThread, GStderr, EVT_CMD_DONE, EVT_CMD_OUTPUT

from web_services.cap_interface import (
WMSCapabilities,
WMTSCapabilities,
OnEarthCapabilities,
)

from gui_core.widgets import GNotebook
from gui_core.widgets import ManageSettingsWidget
from gui_core.wrap import (
Expand All @@ -59,7 +47,13 @@
TreeCtrl,
)

import grass.script as grass
if globalvar.wxPythonPhoenix:
try:
import agw.flatnotebook as FN
except ImportError: # if it's not there locally, try the wxPython lib.
import wx.lib.agw.flatnotebook as FN
else:
import wx.lib.flatnotebook as FN

rinwms_path = os.path.join(os.getenv("GISBASE"), "etc", "r.in.wms")
if rinwms_path not in sys.path:
Expand Down
2 changes: 1 addition & 1 deletion python/grass/docs/conf.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file I'm ok with.

Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

import sys
import os
from datetime import date
import string
from datetime import date
from shutil import copy

# If extensions (or modules to document with autodoc) are in another directory,
Expand Down
1 change: 0 additions & 1 deletion python/grass/pygrass/raster/__init__.py
View file
Open in desktop
echoix marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#
from grass.script import fatal
from grass.exceptions import OpenError

import grass.lib.gis as libgis
import grass.lib.raster as libraster
import grass.lib.rowio as librowio
Expand Down
4 changes: 2 additions & 2 deletions python/grass/pygrass/vector/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from os.path import join, exists
import grass.lib.gis as libgis

libgis.G_gisinit("")
import grass.lib.vector as libvect
import ctypes

libgis.G_gisinit("")

#
# import pygrass modules
#
Expand Down
22 changes: 8 additions & 14 deletions scripts/r.in.wms/wms_drv.py
View file
Open in desktop
echoix marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,16 @@

import socket
import grass.script as grass
import numpy as Numeric
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it was there before, but probably a new PR for this, but it is almost an universal convention across all Python code that numpy is imported as np, like import numpy as np. Why should we be different here?

Probably someone from this repo can jump in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was added a long time ago in 31d721a - perhaps the style of back then? Can @landam remember?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is just some very old practice carried over. Just change that.


from time import sleep
from math import pi, floor
from urllib.error import HTTPError
from http.client import HTTPException
from xml.etree.ElementTree import ParseError
from wms_base import GetEpsg, GetSRSParamVal, WMSBase
from wms_cap_parsers import WMTSCapabilitiesTree, OnEarthCapabilitiesTree
from srs import Srs

try:
from osgeo import gdal
Expand All @@ -30,22 +38,8 @@
)
)

import numpy as Numeric

Numeric.arrayrange = Numeric.arange

from math import pi, floor

from urllib.error import HTTPError
from http.client import HTTPException

from xml.etree.ElementTree import ParseError

from wms_base import GetEpsg, GetSRSParamVal, WMSBase

from wms_cap_parsers import WMTSCapabilitiesTree, OnEarthCapabilitiesTree
from srs import Srs


class WMSDrv(WMSBase):
def _download(self):
Expand Down
Loading