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.

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

Issue984/switch to pylint #986

Open
wants to merge 40 commits into
base: develop
Choose a base branch
from
Open

Issue984/switch to pylint #986

wants to merge 40 commits into from

Conversation

Leahh02
Copy link
Collaborator

@Leahh02 Leahh02 commented Jan 9, 2025

This PR fixes issue #984.

pyproject.toml and setup.cfg have been updated, and pylama.yml has been changed to pylint.yml. I chose pylint 3.2.7 because it supports python >= 3.8 and our current python version is >=3.8.3,<=3.13.0. All of the other recent pylint releases support python >= 3.9.

This PR will be compared with the one I'll make switching to Ruff so we can decide which one to switch to.

@Leahh02 Leahh02 added the WIP (lint-only) Only run the linter in CI, nothing else label Jan 9, 2025
@Leahh02
Copy link
Collaborator Author

Leahh02 commented Jan 13, 2025

Message controls for specific lines and for entire files

For pylama we have some lines where we want to skip linting. We do this by having # noqa at the end of the line. pylint does not follow the same syntax. To skip message codes on a line add # pylint: disable=message_code or # pylint: disable=message_name to the end of the line. For example, to disable R0916 add # pylint: disable=R0916 or # pylint: disable=too-many-boolean-expressions to the end of the line. To skip all of the messages from a category on a specific line add # pylint: disable=category_code. For example, to skip all the refactoring related codes for a line add # pylint: disable=R to the end. We'll have to change the current # noqa lines to the pylint equivalent. There might also be # noqa lines that don't have a pylint error, so we can just remove that.

In addition to skipping specific lines, we also have specific message codes skipped for an entire file. We're doing this in pylama by adding # pylama:ignore=message_code at the bottom of the file. For example, we have # pylama:ignore=W0511,R1732 at the bottom of bee_client.py. The pylint equivalent of this is adding # pylint: disable=message_code or # pylint: disable=message_name at the top of the file. We can also skip an entire category for a file by adding # pylint: disable=category_code at the top. These disable lines must be at the top of the file -- this is different from pylama and ruff where they can be at the bottom or the top (see note about this in PR #987).

Here's documentation about disabling messages in pylint: https://pylint.pycqa.org/en/latest/user_guide/messages/message_control.html

@Leahh02

This comment was marked as outdated.

@Leahh02
Copy link
Collaborator Author

Leahh02 commented Jan 30, 2025

I'm disabling W1309, f-string-without-interpolation, in beeflow/common/parser/parser.py because there was a #noqa there to ignore that error, but I'm not sure why the f that is causing this message code is there.

@Leahh02 Leahh02 removed the WIP (lint-only) Only run the linter in CI, nothing else label Jan 30, 2025
@Leahh02 Leahh02 added the WIP (lint-only) Only run the linter in CI, nothing else label Jan 30, 2025
@Leahh02
Copy link
Collaborator Author

Leahh02 commented Jan 30, 2025

************* Module common.worker.worker
beeflow/common/worker/worker.py:47:49: E0606: Possibly using variable 'crt_driver' before assignment (possibly-used-before-assignment)
************* Module common.worker.lsf_worker
beeflow/common/worker/lsf_worker.py:43:12: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module client.bee_client
beeflow/client/bee_client.py:389:38: E0606: Possibly using variable 'workflow' before assignment (possibly-used-before-assignment)
beeflow/client/bee_client.py:394:28: E0606: Possibly using variable 'wf_tarball' before assignment (possibly-used-before-assignment)
beeflow/client/bee_client.py:704:51: E0606: Possibly using variable 'wf_tarball' before assignment (possibly-used-before-assignment)
************* Module wf_manager.resources.wf_actions
beeflow/wf_manager/resources/wf_actions.py:85:15: E0606: Possibly using variable 'resp' before assignment (possibly-used-before-assignment)
************* Module wf_manager.resources.wf_utils
beeflow/wf_manager/resources/wf_utils.py:189:11: E0606: Possibly using variable 'url' before assignment (possibly-used-before-assignment)
************* Module wf_manager.resources.wf_list
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==data.cwl.bee_workflows.pennant-build.graph_pennant:[10:44]
==data.dockerfiles.pennant-graph.graph_pennant:[10:44]
results = []
for fname in sys.argv[1:]:
    pe_count = 0
    times = []
    with open(fname, encoding='utf-8') as fp:
        for line in fp:
            # Check for the PE count
            m_pe_count = re.match(r'Running on (\d+) MPI PE\(s\)', line)
            if m_pe_count:
                pe_count = int(m_pe_count.group(1))
                continue
            # Check for an End cyle line
            if not line.startswith('End cycle'):
                continue
            _, _, _, wall = line.split(',')
            _, time = wall.split('=')
            time = float(time.strip())
            times.append(time)
    results.append({
        'pe_count': pe_count,
        'average_wall_time': sum(times) / len(times),
    })

# The node counts
x = [str(result['pe_count']) for result in results]
# Average wall for cycle
y = [result['average_wall_time'] for result in results]
fig, ax = plt.subplots()
ax.plot(x, y)
ax.set_title('PENNANT Workflow Run')
ax.set_xlabel('Node count')
ax.set_ylabel('Average wall time for cycle')
# Save to a png file
fig.savefig('graph.png') (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.bdb:[65:80]
==wf_manager.common.wf_db:[48:63]
        try:
            cursor = conn.cursor()
            if params:
                cursor.execute(stmt, params)
            else:
                cursor.execute(stmt)
            result = cursor.fetchall()
        except Error:
            result = None
    return result


def table_exists(db_file, table_name):
    """Return true if a table exists and false if not."""
    stmt = f"SELECT name FROM sqlite_master WHERE type='table' AND name='{table_name}';" (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.bdb:[36:49]
==wf_manager.common.wf_db:[34:47]
        try:
            cursor = conn.cursor()
            if params:
                cursor.execute(stmt, params)
            else:
                cursor.execute(stmt)
            conn.commit()
        except Error as error:
            print(error)


def getone(db_file, stmt, params=None):
    """Run the sql statement on the database and return the result.""" (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.wfm_db:[175:185]
==wf_manager.common.wf_db:[105:115]
                    FOREIGN KEY (workflow_id)
                        REFERENCES workflows (workflow_id)
                            ON DELETE CASCADE
                            ON UPDATE NO ACTION);"""

    info_stmt = """CREATE TABLE IF NOT EXISTS info (
                       id INTEGER PRIMARY KEY,
                       wfm_port INTEGER,
                       tm_port INTEGER,
                       sched_port INTEGER, (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.worker.flux_worker:[69:79]
==common.worker.slurm_worker:[62:71]
        scripts_enabled = task.get_requirement('beeflow:ScriptRequirement', 'enabled',
                                               default=False)
        if scripts_enabled:
            # We use StringIO here to properly break the script up into lines with readlines
            pre_script = io.StringIO(task.get_requirement('beeflow:ScriptRequirement',
                                     'pre_script')).readlines()
            post_script = io.StringIO(task.get_requirement('beeflow:ScriptRequirement',
                                      'post_script')).readlines()

        # Pre commands (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.bdb:[14:25]
==wf_manager.common.wf_db:[12:23]
    conn = None
    try:
        conn = sqlite3.connect(db_file)
        return conn
    except Error as error:
        print(error)
    return conn


def create_table(db_file, stmt):
    """Create a new table in the database.""" (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==data.cwl.cwl_validation.ml-workflow.machine_learning.decision_tree:[20:28]
==data.cwl.cwl_validation.ml-workflow.machine_learning.linear_regression:[16:24]
    X = pickle.load(open('/home/bee/cwl2/MyX.p', 'rb'))
    Y = pickle.load(open('/home/bee/cwl2/MyY.p', 'rb'))

    X = X.values

    print("My pickle X is", X)
    Y = Y.values
    print("My pickle Y is", Y) (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.worker.lsf_worker:[30:39]
==common.worker.slurm_worker:[129:139]
        task_text = self.build_text(task)

        task_script = f'{self.task_save_path(task)}/{task.name}-{task.id}.sh'
        with open(task_script, 'w', encoding='UTF-8') as script_f:
            script_f.write(task_text)
            script_f.close()
        return task_script

    def submit_job(self, script):
        """Worker submits job-returns (job_id, job_state).""" (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.bdb:[50:56]
==wf_manager.common.wf_db:[34:40]
        try:
            cursor = conn.cursor()
            if params:
                cursor.execute(stmt, params)
            else:
                cursor.execute(stmt) (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.bdb:[36:42]
==wf_manager.common.wf_db:[48:54]
        try:
            cursor = conn.cursor()
            if params:
                cursor.execute(stmt, params)
            else:
                cursor.execute(stmt) (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.bdb:[26:35]
==wf_manager.common.wf_db:[24:33]
        try:
            cursor = conn.cursor()
            cursor.execute(stmt)
        except Error as error:
            print(error)


def run(db_file, stmt, params=None):
    """Run the sql statement on the database. Doesn't return anything.""" (duplicate-code)
beeflow/wf_manager/resources/wf_list.py:1:0: R0801: Similar lines in 2 files
==common.db.wfm_db:[157:162]
==wf_manager.common.wf_db:[86:91]
        workflows_stmt = """CREATE TABLE IF NOT EXISTS workflows (
                                id INTEGER PRIMARY KEY,
                                -- Set workflow ID to unique.
                                workflow_id INTEGER UNIQUE,
                                name TEXT, (duplicate-code)

-----------------------------------
Your code has been rated at 9.94/10

@Leahh02
Copy link
Collaborator Author

Leahh02 commented Jan 30, 2025

The only errors left are E0606, W0719, and R0801. I would like some input on these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP (lint-only) Only run the linter in CI, nothing else
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant