Changes to make Scrapy Item class customizable via configuration

[thihara]

No description provided.

[fhamborg]

Hi, thanks for the PR. Could you please elaborate which problem these changes solve, i.e., what would be the use case when one wants to customize the ScrapyItem? Thank you!

[thihara]

Hi,

Sorry I should've explained better.

If I want to modify the plugin to include secondary processing - let's say add a word count or something, the pipeline can't add additional fields to the current Scrapy Item object being moved forward in the pipeline.

This PR is to allow a sub class of the current Scrapy Item class to be substituted via the config.cfg file. This will allow the pipeline to be customized extensively - without forking the project.

[fhamborg]

Thanks for your elaborations. After reviewing the code, I think there are a few things that should be addressed before it can be merged into master.

1. Because in the thihara:item_class branch only the config.cfg but not the config_lib.cfg is changed to contain the new attribute, your changes cannot be used in library mode, but only in CLI mode. I think the feature makes sense in both modes and for consistency thus should be added to lib mode, too.
2. f-strings were used, which required py>=3.6, whereas news-please also runs on py3.5. Could you replace the f-strings with "{}".format()?
   Please let me know if you have any questions and thanks for the work you already put into this.

[fhamborg]

see my other comment

[thihara]

@fhamborg I didn't think there would be much use for setting up an customized pipeline in the library mode. For me using library mode means you would use existing pipeline elements and add your own customizations programmatically, in which case you have complete control anyway.

But I'll defer to you on this and add the configuration to the config_lib.cfg file.

Sorry about the f strings - should've realized that would increase the library supported python version. I'll fix it too.

[fhamborg]

@fhamborg I didn't think there would be much use for setting up an customized pipeline in the library mode. For me using library mode means you would use existing pipeline elements and add your own customizations programmatically, in which case you have complete control anyway.

Thanks for the hint. Do I understand you correctly that the changes you made initially in the PR are for the use case of being in CLI mode and having news-please set up by pypi?

[thihara]

@fhamborg That is correct.

[thihara]

@fhamborg Changes are done. Please review and let me know if there are additional issues.

[thihara]

@fhamborg Is there anything else blocking this PR?

[fhamborg]

Sorry for the delay. Before I can merge this PR, I need to comprehensively test the changes that come with it, as they are partly in core files of news-please.

[thihara]

@fhamborg Understood. Let me know if there's anything I can do to help speed things up.

[fhamborg]

Just a few minors, other than that we can merge. Thanks for your effort!

[fhamborg]

1. please replace does't with doesn't
2. Why this test condition on containing "."? IIRC, is not only some.path.modulename a valid path, but for example only modulename is also a valid import path.

[fhamborg]

Could you please rename the file to a more descriptive name, e.g., class_loader.py? Because currently it only contains the ClassLoader class and nothing else, thus we should go with the more descriptive name.

[thihara]

@fhamborg About no2; the fully qualified name will be module.class at least. The use of a non names-paced class seemed rare enough I didn't see the value in handling that case. Do you disagree?

I'll do the other changes and update the PR soon. Apologies about the long delay.

[thihara]

@fhamborg Changes are done.

[fhamborg]

Thanks for the PR and the subsequent changes you implemented. I will keep this in the master branch for a while and if no issues arise push a new version to Pypi.