diff --git a/.docker/Dockerfile b/.docker/Dockerfile index 7fa7fc1e3ba287611b84d22cd969ef50655f8a8f..876f252299991f2fa4410994b73259c3593c2198 100644 --- a/.docker/Dockerfile +++ b/.docker/Dockerfile @@ -25,7 +25,7 @@ ADD https://gitlab.com/api/v4/projects/13656973/repository/branches/dev \ RUN git clone https://gitlab.com/caosdb/caosdb-pylib.git && \ cd caosdb-pylib && git checkout dev && pip3 install . # At least recommonmark 0.6 required. -RUN pip3 install recommonmark sphinx-rtd-theme +RUN pip3 install -U html2text pycodestyle pylint recommonmark sphinx-rtd-theme COPY . /git RUN rm -r /git/.git \ && mv /git/.docker/pycaosdb.ini /git/integrationtests diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ea5eb78bd8323b1dd7199dc5eb91e899b1d98f81..8ebbefaa39650ddaff45b856a8a4d44a2ac495d1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -104,20 +104,30 @@ cert: script: - cd .docker - CAOSHOSTNAME=caosdb-server ./cert.sh + style: tags: [docker] stage: style image: $CI_REGISTRY_IMAGE - needs: [] + needs: [build-testenv] script: - make style allow_failure: true +linting: + tags: [docker] + stage: style + image: $CI_REGISTRY_IMAGE + needs: [build-testenv] + script: + - make lint + allow_failure: true + unittest: tags: [docker] stage: unittest image: $CI_REGISTRY_IMAGE - needs: [] + needs: [build-testenv] script: - tox diff --git a/Makefile b/Makefile index 52ac04456cf59a24334003d4a0af9055dd3b11ec..d9b182cbd0b17490e9d81b900d6ba8cefadb1b64 100644 --- a/Makefile +++ b/Makefile @@ -36,5 +36,10 @@ unittest: pytest-3 unittests style: + pycodestyle --count src unittests --exclude=swagger_client autopep8 -ar --diff --exit-code --exclude swagger_client . .PHONY: style + +lint: + pylint --unsafe-load-any-extension=y -d all -e E,F --ignore=swagger_client src/caosadvancedtools +.PHONY: lint diff --git a/pylintrc b/pylintrc index 8a12125d4b71d3df5f7866277c41ee15401a4a93..625f83ce950841f7a239538123ef7b5812fc5c5f 100644 --- a/pylintrc +++ b/pylintrc @@ -2,8 +2,18 @@ [FORMAT] # Good variable names which should always be accepted, separated by a comma -good-names=ii,rt - +good-names=ii,rt,df [TYPECHECK] -ignored-modules=etree +# List of module names for which member attributes should not be checked +# (useful for modules/projects where namespaces are manipulated during runtime +# and thus existing member attributes cannot be deduced by static analysis +ignored-modules=etree,h5py,labfolder + +[MASTER] +# TODO: The max_inferred size is necessary for https://github.com/PyCQA/pylint/issues/4577, +# otherwise pandas.read_csv's return value would be inferred as TextFileReader. +init-hook= + import sys; sys.path.extend(["src/caosadvancedtools"]); + import astroid; astroid.context.InferenceContext.max_inferred = 500; + diff --git a/src/caosadvancedtools/cfood.py b/src/caosadvancedtools/cfood.py index 3c2d5408ef4d857f62ce4e908f90c4ffccef4d19..ada76f4b011778ea07fb99729ca3ac457081ad1d 100644 --- a/src/caosadvancedtools/cfood.py +++ b/src/caosadvancedtools/cfood.py @@ -813,7 +813,7 @@ class RowCFood(AbstractCFood): rec.add_property(key, value) -class CMeal(object): +class CMeal(): """ CMeal groups equivalent items and allow their collected insertion. @@ -841,8 +841,16 @@ class CMeal(object): matching_groups = [] def __init__(self): + self.item = None + # FIXME is this only necessary, because of inconsistent use of super().__init__()? + if "match" not in self.__dict__: + self.match = None self.__class__.existing_instances.append(self) + @staticmethod + def get_re(): + raise NotImplementedError("Subclasses must implement this function.") + @classmethod def all_groups_equal(cls, m1, m2): equal = True @@ -878,5 +886,5 @@ class CMeal(object): if match is None: return False - else: - return self.all_groups_equal(match, self.match) + + return self.all_groups_equal(match, self.match) diff --git a/src/caosadvancedtools/cfoods/h5.py b/src/caosadvancedtools/cfoods/h5.py index 6c68edd3668fec957126aa3234a830aab98fcd25..cbf9d0baefa435b71eeaeefe63a9b018faabe7ea 100644 --- a/src/caosadvancedtools/cfoods/h5.py +++ b/src/caosadvancedtools/cfoods/h5.py @@ -124,6 +124,7 @@ class H5CFood(AbstractFileCFood): """CFood which consumes HDF5 files.""" super().__init__(*args, **kwargs) self.h5file = None + self.identifiable_root = None self.root_name = "root" self.hdf5Container = db.Container() self.em = EntityMapping() diff --git a/src/caosadvancedtools/converter/labfolder_api.py b/src/caosadvancedtools/converter/labfolder_api.py index a29d965b1598285105a06871ee1017adfdf4e222..cf57c0155a3b3970834abb2fc1058215ef7ecba8 100644 --- a/src/caosadvancedtools/converter/labfolder_api.py +++ b/src/caosadvancedtools/converter/labfolder_api.py @@ -28,7 +28,7 @@ import time import html2text import caosdb as db -from labfolder.connection import configure_connection +from labfolder.connection import configure_connection # pylint: disable=import-error class Importer(object): diff --git a/src/caosadvancedtools/models/parser.py b/src/caosadvancedtools/models/parser.py index 40a61c6c9dbf3273c0287827cc68974d7be716cf..48fc1e722c4ce9e888b8b80dbb5f29595c2f6b26 100644 --- a/src/caosadvancedtools/models/parser.py +++ b/src/caosadvancedtools/models/parser.py @@ -566,9 +566,9 @@ class Parser(object): db.BOOLEAN]: if is_list: - value.datatype = db.LIST(db.__getattribute__(dtype)) + value.datatype = db.LIST(db.__getattribute__(dtype)) # pylint: disable=no-member else: - value.datatype = db.__getattribute__(dtype) + value.datatype = db.__getattribute__(dtype) # pylint: disable=no-member continue @@ -757,7 +757,7 @@ class JsonSchemaParser(Parser): def _treat_list(self, elt: dict, name: str): # @review Timm Fitschen 2022-02-30 - if not "items" in elt: + if "items" not in elt: raise JsonSchemaDefinitionError( f"The definition of the list items is missing in {elt}.") items = elt["items"] @@ -767,7 +767,7 @@ class JsonSchemaParser(Parser): datatype = db.LIST(self._get_atomic_datatype(items)) return db.Property(name=name, datatype=datatype), False if items["type"] == "object": - if not "title" in items or self._stringify(items["title"]) == name: + if "title" not in items or self._stringify(items["title"]) == name: # Property is RecordType return self._treat_record_type(items, name), True else: diff --git a/src/caosadvancedtools/pandoc_header_tools.py b/src/caosadvancedtools/pandoc_header_tools.py index 262defd2e46ea1a6fbe80ab6c476bb8f311cc9a5..e746a26ac19c00de4ee7785399ef98478472340c 100644 --- a/src/caosadvancedtools/pandoc_header_tools.py +++ b/src/caosadvancedtools/pandoc_header_tools.py @@ -136,10 +136,10 @@ it is not at the beginning, it must be preceded by a blank line. # If a header section was found: if state == 2: headerlines = [] - for l in textlines[found_1:found_2]: - l = l.replace("\t", " ") - l = l.rstrip() - headerlines.append(l) + for line in textlines[found_1:found_2]: + line = line.replace("\t", " ") + line = line.rstrip() + headerlines.append(line) # try: try: yaml_part = yaml.load("\n".join(headerlines), Loader=yaml.BaseLoader) @@ -156,7 +156,7 @@ it is not at the beginning, it must be preceded by a blank line. else: print("Adding header in: {fn}".format(fn=filename)) add_header(filename) - return _get_header(filename) + return get_header(filename) def save_header(filename, header_data): diff --git a/src/caosadvancedtools/scifolder/simulation_cfood.py b/src/caosadvancedtools/scifolder/simulation_cfood.py index ae129e6a69ce25c6698b98124e81f8bc2921b472..c8f23f1485d7a1f64dcd940552051d2e1ec5bb07 100644 --- a/src/caosadvancedtools/scifolder/simulation_cfood.py +++ b/src/caosadvancedtools/scifolder/simulation_cfood.py @@ -88,22 +88,22 @@ class SimulationCFood(AbstractFileCFood, WithREADME): self.to_be_updated, datatype=db.LIST(db.REFERENCE)) - if SOURCES.key in self.header: + if SOURCES.key in self.header: # pylint: disable=unsupported-membership-test reference_records_corresponding_to_files( record=self.simulation, recordtypes=["Experiment", "Publication", "Simulation", "Analysis"], - globs=get_glob(self.header[SOURCES.key]), + globs=get_glob(self.header[SOURCES.key]), # pylint: disable=unsubscriptable-object property_name=dm.sources, path=self.crawled_path, to_be_updated=self.to_be_updated) self.reference_files_from_header(record=self.simulation) - if REVISIONOF.key in self.header: + if REVISIONOF.key in self.header: # pylint: disable=unsupported-membership-test reference_records_corresponding_to_files( record=self.simulation, - recordtypes=[dm.Software], + recordtypes=[dm.Software], # pylint: disable=no-member property_name=dm.revisionOf, - globs=get_glob(self.header[dm.revisionOf]), + globs=get_glob(self.header[dm.revisionOf]), # pylint: disable=unsubscriptable-object path=self.crawled_path, to_be_updated=self.to_be_updated) diff --git a/src/caosadvancedtools/scifolder/withreadme.py b/src/caosadvancedtools/scifolder/withreadme.py index 8a63e1f6d90ed4e78d01f76393cc72982cdc79d4..e1968ba49799827467c7ef93a7070b7f090010fb 100644 --- a/src/caosadvancedtools/scifolder/withreadme.py +++ b/src/caosadvancedtools/scifolder/withreadme.py @@ -121,12 +121,12 @@ class WithREADME(object): @property def header(self): if self._header is None: - if self.crawled_path.lower().endswith(".md"): + if self.crawled_path.lower().endswith(".md"): # pylint: disable=no-member self._header = get_md_header( - fileguide.access(self.crawled_path)) - elif self.crawled_path.lower().endswith(".xlsx"): + fileguide.access(self.crawled_path)) # pylint: disable=no-member + elif self.crawled_path.lower().endswith(".xlsx"): # pylint: disable=no-member self._header = get_xls_header( - fileguide.access(self.crawled_path)) + fileguide.access(self.crawled_path)) # pylint: disable=no-member else: raise RuntimeError("Readme format not recognized.") self.convert_win_paths() @@ -145,7 +145,7 @@ class WithREADME(object): globs = get_glob(self.header[field.key]) files = get_files_referenced_by_field( - globs, prefix=os.path.dirname(self.crawled_path)) + globs, prefix=os.path.dirname(self.crawled_path)) # pylint: disable=no-member description = [get_description(val) for val in self.header[field.key]] @@ -160,7 +160,7 @@ class WithREADME(object): LOGGER.warn("ATTENTION: the field {} does not reference any " "known files".format(field.key)) - self.attached_filenames.extend(flat_list) + self.attached_filenames.extend(flat_list) # pylint: disable=no-member def convert_path(self, el): """ converts the path in el to unix type @@ -185,7 +185,7 @@ class WithREADME(object): return win_path_converter(el) def convert_win_paths(self): - for field in self.win_paths: + for field in self.win_paths: # pylint: disable=no-member if field in self.header: if isinstance(self.header[field], list): @@ -245,7 +245,7 @@ class WithREADME(object): references[ref_type], record, ref_type, - to_be_updated=self.to_be_updated, + to_be_updated=self.to_be_updated, # pylint: disable=no-member ) def reference_included_records(self, record, fields, to_be_updated): @@ -255,16 +255,16 @@ class WithREADME(object): for field in fields: - if field.key not in self.header: + if field.key not in self.header: # pylint: disable=no-member continue included = [] - for item in self.header[field.key]: + for item in self.header[field.key]: # pylint: disable=no-member if INCLUDE.key in item: try: included.extend( get_entity_ids_from_include_file( - os.path.dirname(self.crawled_path), + os.path.dirname(self.crawled_path), # pylint: disable=no-member item[INCLUDE.key])) except ValueError: al = logging.getLogger("caosadvancedtools") diff --git a/src/caosadvancedtools/serverside/generic_analysis.py b/src/caosadvancedtools/serverside/generic_analysis.py index 66bec8a77e55709434b4285699e2cc2f8f804894..85d0c860df75fce205c5eaad77731fc04eee9e40 100644 --- a/src/caosadvancedtools/serverside/generic_analysis.py +++ b/src/caosadvancedtools/serverside/generic_analysis.py @@ -210,5 +210,4 @@ def main(): if __name__ == "__main__": - args = _parse_arguments() - sys.exit(main(args)) + sys.exit(main()) diff --git a/src/caosadvancedtools/serverside/helper.py b/src/caosadvancedtools/serverside/helper.py index 19efc9ed2b3e99e17eb28f5c87b0a6dbc0c47499..ba75739e0fdc0a83f235db6920471afb196f4246 100644 --- a/src/caosadvancedtools/serverside/helper.py +++ b/src/caosadvancedtools/serverside/helper.py @@ -390,11 +390,11 @@ def send_mail(from_addr, to, subject, body, cc=None, bcc=None, else: caosdb_config = db.configuration.get_config() - if not "Misc" in caosdb_config or not "sendmail" in caosdb_config["Misc"]: + if "Misc" not in caosdb_config or "sendmail" not in caosdb_config["Misc"]: err_msg = ("No sendmail executable configured. " "Please configure `Misc.sendmail` " "in your pycaosdb.ini.") - raise db.ConfigurationException(err_msg) + raise db.ConfigurationError(err_msg) sendmail = caosdb_config["Misc"]["sendmail"] # construct sendmail command diff --git a/src/caosadvancedtools/table_export.py b/src/caosadvancedtools/table_export.py index bed0edc97a794dd83b2bdd7b1c0449c710c18d3f..056207a76fa01357e2269cd4cb8e9a09905d5d90 100644 --- a/src/caosadvancedtools/table_export.py +++ b/src/caosadvancedtools/table_export.py @@ -308,7 +308,7 @@ class BaseTableExporter(object): " was specified but no record is given." ) else: - if not "selector" in d: + if "selector" not in d: d["selector"] = d[QUERY].strip().split(" ")[1] # guess find function and insert if existing else: diff --git a/unittests/create_filetree.py b/unittests/create_filetree.py index 6f95618dbc834c3bc140163efdc90aa51c8d5248..f80b9681163859027bb8f8c7cd6b1387bf2d378d 100644 --- a/unittests/create_filetree.py +++ b/unittests/create_filetree.py @@ -42,8 +42,6 @@ def main(folder, dry=True): if not dry: os.mkdir(series_path) for date in [datetime.today()-timedelta(days=i)-timedelta(weeks=50*ii) for i in range(10)]: - #import IPython - # IPython.embed() exp_path = os.path.join(series_path, "Exp_"+str(date.date())) print("Exp: "+os.path.basename(exp_path)) if not dry: diff --git a/unittests/test_cfood.py b/unittests/test_cfood.py index f5125166106c4bace21121d58a025886f9b132b9..7055bc7c51962c0cbc487f29bcdacb391218a7d3 100644 --- a/unittests/test_cfood.py +++ b/unittests/test_cfood.py @@ -48,13 +48,14 @@ class ExampleCFoodMeal(AbstractFileCFood, CMeal): CMeal.__init__(self) @classmethod - def match_item(cls, item): + def match_item(cls, path): """ standard match_match, but returns False if a suitable cfood exists """ - if cls.has_suitable_cfood(item): + print(path) + if cls.has_suitable_cfood(path): return False - return re.match(cls.get_re(), item) is not None + return re.match(cls.get_re(), path) is not None def looking_for(self, crawled_file): """ standard looking_for, but returns True if the file matches all diff --git a/unittests/test_yaml_model_parser.py b/unittests/test_yaml_model_parser.py index 01730cdb1690c7c4a917475d10c9035177fb58b7..a9f072b754618e38237cbf70e74c7944551f1045 100644 --- a/unittests/test_yaml_model_parser.py +++ b/unittests/test_yaml_model_parser.py @@ -291,12 +291,12 @@ A: """ model = parse_model_from_string(modeldef) self.assertEqual(len(model), 2) - for key in model.keys(): + for key, value in model.items(): if key == "A": - self.assertTrue(isinstance(model[key], db.RecordType)) + self.assertTrue(isinstance(value, db.RecordType)) elif key == "ref": - self.assertTrue(isinstance(model[key], db.Property)) - self.assertEqual(model[key].datatype, "LIST<A>") + self.assertTrue(isinstance(value, db.Property)) + self.assertEqual(value.datatype, "LIST<A>") class ExternTest(unittest.TestCase): @@ -337,7 +337,7 @@ A: # parse_str(string) with self.assertRaises(YamlDefinitionError) as yde: parse_str(string) - assert("line {}".format(line) in yde.exception.args[0]) + assert "line {}".format(line) in yde.exception.args[0] def test_define_role(): @@ -366,11 +366,11 @@ D: role: RecordType """ entities = parse_model_from_string(model) - for l, ent in (("A", "Record"), ("b", "Property"), - ("C", "RecordType"), ("D", "RecordType")): - assert l in entities - assert isinstance(entities[l], getattr(db, ent)) - assert entities[l].role == ent + for name, ent in (("A", "Record"), ("b", "Property"), + ("C", "RecordType"), ("D", "RecordType")): + assert name in entities + assert isinstance(entities[name], getattr(db, ent)) + assert entities[name].role == ent assert entities["A"].parents[0].name == "C" assert entities["A"].name == "A"