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

Remove all files from the pre-commit exclude list #4196

Merged
merged 1 commit into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 25 additions & 20 deletions .ci/workchains.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
# For further information please visit http://www.aiida.net #
###########################################################################
# pylint: disable=invalid-name
"""Work chain implementations for testing purposes."""
from aiida.common import AttributeDict
from aiida.engine import calcfunction, workfunction, WorkChain, ToContext, append_, while_, ExitCode
from aiida.engine import BaseRestartWorkChain, process_handler, ProcessHandlerReport
from aiida.engine.persistence import ObjectLoader
from aiida.orm import Int, List, Str
from aiida.plugins import CalculationFactory


ArithmeticAddCalculation = CalculationFactory('arithmetic.add')


Expand Down Expand Up @@ -54,15 +54,15 @@ def setup(self):
def sanity_check_not_too_big(self, node):
"""My puny brain cannot deal with numbers that I cannot count on my hand."""
if node.is_finished_ok and node.outputs.sum > 10:
return ProcessHandlerReport(True, self.exit_codes.ERROR_TOO_BIG)
return ProcessHandlerReport(True, self.exit_codes.ERROR_TOO_BIG) # pylint: disable=no-member

@process_handler(priority=460, enabled=False)
def disabled_handler(self, node):
def disabled_handler(self, node): # pylint: disable=unused-argument
"""By default this is not enabled and so should never be called, irrespective of exit codes of sub process."""
return ProcessHandlerReport(True, self.exit_codes.ERROR_ENABLED_DOOM)
return ProcessHandlerReport(True, self.exit_codes.ERROR_ENABLED_DOOM) # pylint: disable=no-member

@process_handler(priority=450, exit_codes=ExitCode(1000, 'Unicorn encountered'))
def a_magic_unicorn_appeared(self, node):
def a_magic_unicorn_appeared(self, node): # pylint: disable=no-self-argument,no-self-use
"""As we all know unicorns do not exist so we should never have to deal with it."""
raise RuntimeError('this handler should never even have been called')

Expand All @@ -78,30 +78,24 @@ class NestedWorkChain(WorkChain):
"""
Nested workchain which creates a workflow where the nesting level is equal to its input.
"""

@classmethod
def define(cls, spec):
super().define(spec)
spec.input('inp', valid_type=Int)
spec.outline(
cls.do_submit,
cls.finalize
)
spec.outline(cls.do_submit, cls.finalize)
spec.output('output', valid_type=Int, required=True)

def do_submit(self):
if self.should_submit():
self.report('Submitting nested workchain.')
return ToContext(
workchain=append_(self.submit(
NestedWorkChain,
inp=self.inputs.inp - 1
))
)
return ToContext(workchain=append_(self.submit(NestedWorkChain, inp=self.inputs.inp - 1)))

def should_submit(self):
return int(self.inputs.inp) > 0

def finalize(self):
"""Attach the outputs."""
if self.should_submit():
self.report('Getting sub-workchain output.')
sub_workchain = self.ctx.workchain[0]
Expand All @@ -112,15 +106,13 @@ def finalize(self):


class SerializeWorkChain(WorkChain):
"""Work chain that serializes inputs."""

@classmethod
def define(cls, spec):
super().define(spec)

spec.input(
'test',
valid_type=Str,
serializer=lambda x: Str(ObjectLoader().identify_object(x))
)
spec.input('test', valid_type=Str, serializer=lambda x: Str(ObjectLoader().identify_object(x)))

spec.outline(cls.echo)
spec.outputs.dynamic = True
Expand All @@ -130,6 +122,8 @@ def echo(self):


class NestedInputNamespace(WorkChain):
"""Work chain with nested namespace."""

@classmethod
def define(cls, spec):
super().define(spec)
Expand All @@ -143,6 +137,8 @@ def do_echo(self):


class ListEcho(WorkChain):
"""Work chain that simply echos a `List` input."""

@classmethod
def define(cls, spec):
super().define(spec)
Expand All @@ -157,6 +153,8 @@ def do_echo(self):


class DynamicNonDbInput(WorkChain):
"""Work chain with dynamic non_db inputs."""

@classmethod
def define(cls, spec):
super().define(spec)
Expand All @@ -172,6 +170,8 @@ def do_test(self):


class DynamicDbInput(WorkChain):
"""Work chain with dynamic input namespace."""

@classmethod
def define(cls, spec):
super().define(spec)
Expand All @@ -186,6 +186,8 @@ def do_test(self):


class DynamicMixedInput(WorkChain):
"""Work chain with dynamic mixed input."""

@classmethod
def define(cls, spec):
super().define(spec)
Expand All @@ -194,6 +196,7 @@ def define(cls, spec):
spec.outline(cls.do_test)

def do_test(self):
"""Run the test."""
input_non_db = self.inputs.namespace.inputs['input_non_db']
input_db = self.inputs.namespace.inputs['input_db']
assert isinstance(input_non_db, int)
Expand All @@ -206,6 +209,7 @@ class CalcFunctionRunnerWorkChain(WorkChain):
"""
WorkChain which calls an InlineCalculation in its step.
"""

@classmethod
def define(cls, spec):
super().define(spec)
Expand All @@ -223,6 +227,7 @@ class WorkFunctionRunnerWorkChain(WorkChain):
"""
WorkChain which calls a workfunction in its step
"""

@classmethod
def define(cls, spec):
super().define(spec)
Expand Down
103 changes: 11 additions & 92 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,104 +8,16 @@ repos:
- id: mixed-line-ending
- id: trailing-whitespace

- repo: https://github.com/PyCQA/pylint
rev: pylint-2.5.2
hooks:
- id: pylint
language: system
exclude: &exclude_files >
(?x)^(
.ci/workchains.py|
aiida/backends/djsite/queries.py|
aiida/backends/djsite/db/models.py|
aiida/backends/djsite/db/migrations/0001_initial.py|
aiida/backends/djsite/db/migrations/0002_db_state_change.py|
aiida/backends/djsite/db/migrations/0003_add_link_type.py|
aiida/backends/djsite/db/migrations/0004_add_daemon_and_uuid_indices.py|
aiida/backends/djsite/db/migrations/0005_add_cmtime_indices.py|
aiida/backends/djsite/db/migrations/0006_delete_dbpath.py|
aiida/backends/djsite/db/migrations/0007_update_linktypes.py|
aiida/backends/djsite/db/migrations/0008_code_hidden_to_extra.py|
aiida/backends/djsite/db/migrations/0009_base_data_plugin_type_string.py|
aiida/backends/djsite/db/migrations/0010_process_type.py|
aiida/backends/djsite/db/migrations/0011_delete_kombu_tables.py|
aiida/backends/djsite/db/migrations/0012_drop_dblock.py|
aiida/backends/djsite/db/migrations/0013_django_1_8.py|
aiida/backends/djsite/db/migrations/0014_add_node_uuid_unique_constraint.py|
aiida/backends/djsite/db/migrations/0016_code_sub_class_of_data.py|
aiida/backends/djsite/db/migrations/0017_drop_dbcalcstate.py|
aiida/backends/sqlalchemy/migrations/versions/0aebbeab274d_base_data_plugin_type_string.py|
aiida/backends/sqlalchemy/migrations/versions/35d4ee9a1b0e_code_hidden_attr_to_extra.py|
aiida/backends/sqlalchemy/migrations/versions/59edaf8a8b79_adding_indexes_and_constraints_to_the_.py|
aiida/backends/sqlalchemy/migrations/versions/6c629c886f84_process_type.py|
aiida/backends/sqlalchemy/migrations/versions/70c7d732f1b2_delete_dbpath.py|
aiida/backends/sqlalchemy/migrations/versions/89176227b25_add_indexes_to_dbworkflowdata_table.py|
aiida/backends/sqlalchemy/migrations/versions/a514d673c163_drop_dblock.py|
aiida/backends/sqlalchemy/migrations/versions/a6048f0ffca8_update_linktypes.py|
aiida/backends/sqlalchemy/migrations/versions/e15ef2630a1b_initial_schema.py|
aiida/backends/sqlalchemy/migrations/versions/f9a69de76a9a_delete_kombu_tables.py|
aiida/backends/sqlalchemy/migrations/versions/62fe0d36de90_add_node_uuid_unique_constraint.py|
aiida/backends/sqlalchemy/migrations/versions/a603da2cc809_code_sub_class_of_data.py|
aiida/backends/sqlalchemy/migrations/versions/162b99bca4a2_drop_dbcalcstate.py|
aiida/backends/sqlalchemy/models/computer.py|
aiida/backends/sqlalchemy/models/settings.py|
aiida/backends/sqlalchemy/models/node.py|
aiida/backends/utils.py|
aiida/common/datastructures.py|
aiida/engine/daemon/execmanager.py|
aiida/engine/processes/calcjobs/tasks.py|
aiida/orm/querybuilder.py|
aiida/orm/nodes/data/array/bands.py|
aiida/orm/nodes/data/array/projection.py|
aiida/orm/nodes/data/array/xy.py|
aiida/orm/nodes/data/code.py|
aiida/orm/nodes/data/orbital.py|
aiida/orm/nodes/data/remote.py|
aiida/orm/nodes/data/structure.py|
aiida/orm/utils/remote.py|
aiida/parsers/plugins/arithmetic/add.py|
aiida/parsers/plugins/templatereplacer/doubler.py|
aiida/parsers/plugins/templatereplacer/__init__.py|
aiida/plugins/entry.py|
aiida/plugins/info.py|
aiida/plugins/registry.py|
aiida/tools/data/array/kpoints/legacy.py|
aiida/tools/data/array/kpoints/seekpath.py|
aiida/tools/data/__init__.py|
aiida/tools/dbexporters/__init__.py|
aiida/tools/dbimporters/baseclasses.py|
aiida/tools/dbimporters/__init__.py|
aiida/tools/dbimporters/plugins/cod.py|
aiida/tools/dbimporters/plugins/icsd.py|
aiida/tools/dbimporters/plugins/__init__.py|
aiida/tools/dbimporters/plugins/mpds.py|
aiida/tools/dbimporters/plugins/mpod.py|
aiida/tools/dbimporters/plugins/nninc.py|
aiida/tools/dbimporters/plugins/oqmd.py|
aiida/tools/dbimporters/plugins/pcod.py|
docs/.*|
examples/.*|
tests/engine/test_work_chain.py|
tests/schedulers/test_direct.py|
tests/schedulers/test_lsf.py|
tests/schedulers/test_pbspro.py|
tests/schedulers/test_sge.py|
tests/schedulers/test_torque.py|
tests/sphinxext/workchain_source/conf.py|
tests/sphinxext/workchain_source_broken/conf.py|
tests/transports/test_all_plugins.py|
tests/transports/test_local.py|
tests/transports/test_ssh.py|
tests/test_dataclasses.py|
)$

- repo: https://github.com/pre-commit/mirrors-yapf
rev: v0.30.0
hooks:
- id: yapf
name: yapf
types: [python]
exclude: *exclude_files
exclude: &exclude_files >
(?x)^(
docs/.*|
)$
args: ['-i']

- repo: https://github.com/pre-commit/mirrors-mypy
Expand All @@ -120,6 +32,13 @@ repos:
)$

- repo: local

hooks:
- id: pylint
name: pylint
language: system
exclude: *exclude_files

hooks:
Comment on lines +36 to 42
Copy link
Member

@ltalirz ltalirz Jul 11, 2020

Choose a reason for hiding this comment

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

@sphuber I didn't review this PR but I think you might have just disabled pylint there (hooks key used twice)...

Copy link
Member

Choose a reason for hiding this comment

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

The configured hook doesn't actually work - you'll need to add

entry: pylint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads-up @ltalirz ! That is indeed a required key but normally that should raise a an error when the file is parsed

An error has occurred: InvalidConfigError: 
==> File .pre-commit-config.yaml
==> At Config()
==> At key: repos
==> At Repository(repo='local')
==> At key: hooks
==> At Hook(id='pylint')
=====> Missing required key: entry

The bigger problem is that I accidentally defined the hooks key twice. The first one, containing the pylint config simply gets overridden, which is why pylint is never actually run. I will create a PR to fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See PR #4258

Copy link
Member

Choose a reason for hiding this comment

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

The bigger problem is that I accidentally defined the hooks key twice.

Right. That's what I wrote above, no? ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You did, but not in the original post. I read it on my email, which only ever send the first version. So you edit didn't show up Guess I could have noticed when responding here though :)

- id: dm-generate-all
name: Update all requirements files
Expand Down
3 changes: 2 additions & 1 deletion .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ disable=bad-continuation,
import-outside-toplevel,
cyclic-import,
duplicate-code,
too-few-public-methods
too-few-public-methods,
inconsistent-return-statements

# Enable the message, report, category or checker with the given id(s). You can
# either give multiple identifier separated by comma (,) or put this option
Expand Down
12 changes: 7 additions & 5 deletions aiida/backends/djsite/db/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
# For further information on the license, see the LICENSE.txt file #
# For further information please visit http://www.aiida.net #
###########################################################################

# pylint: disable=invalid-name
"""Database migration."""
from django.db import models, migrations
import django.db.models.deletion
import django.utils.timezone
Expand All @@ -19,6 +20,7 @@


class Migration(migrations.Migration):
"""Database migration."""

dependencies = [
('auth', '0001_initial'),
Expand Down Expand Up @@ -53,8 +55,8 @@ class Migration(migrations.Migration):
'is_active',
models.BooleanField(
default=True,
help_text=
'Designates whether this user should be treated as active. Unselect this instead of deleting accounts.'
help_text='Designates whether this user should be treated as active. Unselect this instead of '
'deleting accounts.'
)
),
('date_joined', models.DateTimeField(default=django.utils.timezone.now)),
Expand All @@ -65,8 +67,8 @@ class Migration(migrations.Migration):
related_name='user_set',
to='auth.Group',
blank=True,
help_text=
'The groups this user belongs to. A user will get all permissions granted to each of his/her group.',
help_text='The groups this user belongs to. A user will get all permissions granted to each of '
'his/her group.',
verbose_name='groups'
)
),
Expand Down
Loading