Create a DCAT-AP Switzerland I14Y RDF Harvester #98
Conversation
kovalch
force-pushed
the
feat/develop_i14y_harvester
branch
from
d2da3db
to
653f0ba
Compare
There was a problem hiding this comment.
Hey, it looks like you just copied and renamed the file harvesters.py and added a couple of bits of logic to the new harvester class, while leaving the rest of the code just the same. Instead of this, could you make the new class SwissDCATI14YRDFHarvester inherit from SwissDCATRDFHarvester, and then only make the adjustments to the logic that are needed? That would be cleaner and easier to maintain.
Some of the comments I left here might not make total sense, because I wrote them assuming that the new class did inherit from SwissDCATRDFHarvester, before realising that I was wrong and it was just a copy.
|
|
||
| return url, [] | ||
|
|
||
| def _get_guid(self, dataset_dict, source_url=None): # noqa |
There was a problem hiding this comment.
Why do we need # noqa here?
There was a problem hiding this comment.
to long line, # noqa is pretty often used in this script
There was a problem hiding this comment.
we can adapt style of the harvesters.py separately.
is it fine for you?
There was a problem hiding this comment.
That line is 56 characters long - # noqa is not needed there because of line length.
It's OK for me to leave the existing code as it is, but please don't introduce overly long lines and other mess into the new code you're adding here. :)
There was a problem hiding this comment.
hehe :) this is a good idea not to bring other mess into the new code.
Will remove # noqa here
There was a problem hiding this comment.
hmm, interesting, when I take it away I get
C901 'SwissDCATRDFHarvester._get_guid' is too complex (16)..
I14Y harvester that adapt the identifier
kovalch
force-pushed
the
feat/develop_i14y_harvester
branch
from
de35586
to
9252c82
Compare
… template