pos: update pos spider #160

spirosdelviniotis · 2017-08-10T14:25:01Z

Description

Signed-off-by: Spyridon Delviniotis [email protected]

Related Issue

Closes #159
On top of #155

Motivation and Context

Checklist:

I have all the information that I need (if not, move to RFC and look for it).
I linked the related issue(s) in the corresponding commit logs.
I wrote good commit log messages.
My code follows the code style of this project.
I've added any new docs if API/utils methods were added.
I have updated the existing documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

david-caro · 2017-08-10T17:50:35Z

hepcrawl/spiders/pos_spider.py


    Example:
        ::

-            $ scrapy crawl PoS -a source_file=file://`pwd`/tests/responses/pos/sample_pos_record.xml
+            $ scrapy crawl PoS -a source_file=file://`pwd`/tests/unit/responses/pos/
+            sample_pos_record.xml


Here you should use something like:

$ my huge \\ big long \\ command

So then it renders nice in the page too and can be copied and pasted to the console, otherwise you get a huge command that goes out of the screen.

david-caro · 2017-08-10T17:55:33Z

hepcrawl/spiders/pos_spider.py

    """
    name = 'PoS'
-    pos_base_url = "https://pos.sissa.it/contribution?id="
+    conference_paper_url = "https://pos.sissa.it/contribution?id="


This name is very confusing, usually for this kind of 'base' url thingies you use the 'base' word, like base_conference_papers_url.

And all caps... though that is not used other other stuff here, so maybe leave the capitalization for a future refactor.

david-caro · 2017-08-10T17:58:14Z

tests/unit/test_pos.py

@@ -35,8 +41,13 @@ def scrape_pos_page_body():


 @pytest.fixture


you can make this fixture 'session', so it does not load it from disk every time.

david-caro · 2017-08-21T12:37:48Z

hepcrawl/spiders/pos_spider.py

-                request.meta["record"] = record.extract()
+                request.meta['url'] = response.url
+                request.meta['record'] = record.extract()
+                request.meta['identifier'] = identifier
                yield request


for record in records: conf_paper_request = get_conf_paper( item_builder_method=build_conf_paper, ... ) # Extracts the pdf from the paper url and builds the conference paper in a callback yield conf_paper_request conf_proceedings_requesw = get_conf_proceedings(...) # Extracts th url and builds the conference paper in a callback yield conf_proceedings_request

david-caro · 2017-08-21T12:44:21Z

tests/functional/pos/test_pos.py

@@ -50,6 +50,7 @@ def set_up_oai_environment():
        'CRAWLER_ARGUMENTS': {
            'source_file': 'file://' + package_location,
            'base_conference_paper_url': 'https://server.local/contribution?id=',
+            'base_proceedings_url': 'https://server.local/cgi-bin/reader/conf.cgi?confid=',


s/setup_up_oai_environment/setup_environment/

david-caro · 2017-08-22T11:34:20Z

hepcrawl/spiders/pos_spider.py

@@ -96,22 +96,22 @@ def parse(self, response):
        response.selector.remove_namespaces()
        records = response.selector.xpath('.//record')
        for record in records:


probably this should be:

for record_xml_selector in record_xml_selectors:

david-caro · 2017-08-22T11:36:01Z

hepcrawl/spiders/pos_spider.py

@@ -277,7 +278,7 @@ def build_conference_proceedings_item(
        record.add_value('journal_title', 'PoS')
        record.add_value(
            'journal_volume',
-            self._get_journal_volume(pos_id=pos_id),
+            self._get_journal_volume(identifier=pos_id),


identifier is too generic, it's a pos internal identifier? a pos external one? orcid? inspire? what?

david-caro · 2017-08-22T11:38:56Z

tests/functional/pos/test_pos.py

@@ -32,7 +32,7 @@ def override_generated_fields(record):


 @pytest.fixture(scope="function")
-def set_up_oai_environment():
+def set_up_environment():


This is actually a bad name, using a verb set_up hints that the function does something (has side effects).
In this case, it should probably be called something like configuration or similar, as what it does, is give back a configuration.

Another option is to have two fixtures instead of one, one with the wait, and the other with the config.

spirosdelviniotis · 2017-08-22T16:05:12Z

Build fixed! 😄

Signed-off-by: Spiros Delviniotis <[email protected]> Signed-off-by: David Caro <[email protected]>

Signed-off-by: Spiros Delviniotis <[email protected]>

Addresses inspirehep#159 Signed-off-by: Spiros Delviniotis <[email protected]>

Signed-off-by: David Caro <[email protected]>

Addresses inspirehep#159 Signed-off-by: Spiros Delviniotis <[email protected]>

Signed-off-by: Spiros Delviniotis <[email protected]>

Signed-off-by: David Caro <[email protected]>

spirosdelviniotis added the Status: WIP label Aug 10, 2017

spirosdelviniotis added this to the Desy spider & PoS specification milestone Aug 10, 2017

spirosdelviniotis self-assigned this Aug 10, 2017

spirosdelviniotis force-pushed the hepcrawl_pos_spider branch from 5f8fe7f to f0f9cc3 Compare August 10, 2017 14:31

spirosdelviniotis changed the title ~~Hepcrawl pos spider~~ pos: update pos spider Aug 10, 2017

spirosdelviniotis requested a review from david-caro August 10, 2017 14:32

spirosdelviniotis force-pushed the hepcrawl_pos_spider branch 2 times, most recently from 47b036d to 4044840 Compare August 10, 2017 14:38

david-caro reviewed Aug 10, 2017

View reviewed changes

spirosdelviniotis force-pushed the hepcrawl_pos_spider branch from 4044840 to 1d6e8e8 Compare August 11, 2017 07:20

spirosdelviniotis force-pushed the hepcrawl_pos_spider branch 2 times, most recently from 8e67b40 to 63fd144 Compare August 21, 2017 11:49

david-caro reviewed Aug 21, 2017

View reviewed changes

david-caro force-pushed the hepcrawl_pos_spider branch from ebe4380 to fc4ba5b Compare August 21, 2017 19:13

david-caro reviewed Aug 22, 2017

View reviewed changes

spirosdelviniotis force-pushed the hepcrawl_pos_spider branch 4 times, most recently from 0a33a49 to 8320ade Compare August 22, 2017 15:59

spirosdelviniotis added Status: Ready for review and removed Status: WIP labels Aug 22, 2017

david-caro self-assigned this Sep 20, 2017

david-caro added Status: WIP and removed Status: Ready for review labels Sep 20, 2017

david-caro force-pushed the hepcrawl_pos_spider branch from 8320ade to 16d2cd7 Compare September 20, 2017 19:04

david-caro force-pushed the hepcrawl_pos_spider branch from 16d2cd7 to 8cd9ca1 Compare October 28, 2017 19:40

ghost added the in progress label Oct 28, 2017

david-caro force-pushed the hepcrawl_pos_spider branch from 8cd9ca1 to c60a4e5 Compare October 29, 2017 15:28

spirosdelviniotis and others added 14 commits November 1, 2017 14:57

pos: first implementation

92d420f

Signed-off-by: Spiros Delviniotis <[email protected]> Signed-off-by: David Caro <[email protected]>

pos: add functional test

81735b8

Signed-off-by: Spiros Delviniotis <[email protected]>

pos: add proceedings support

6c6e4ba

Addresses inspirehep#159 Signed-off-by: Spiros Delviniotis <[email protected]>

pos: refactor

9f5e750

Signed-off-by: David Caro <[email protected]>

pos: add proceedings support

a98cfb2

Addresses inspirehep#159 Signed-off-by: Spiros Delviniotis <[email protected]>

pos: fix spider

21bdc50

Signed-off-by: Spiros Delviniotis <[email protected]>

docker: added proper health dependencies

fcbd1d6

Signed-off-by: David Caro <[email protected]>

middleware: added some extra debug logs

38b60d2

Signed-off-by: David Caro <[email protected]>

pos: fix title and test cleanups

d9c64b1

Signed-off-by: David Caro <[email protected]>

pos: remove fft

c98af1f

Signed-off-by: David Caro <[email protected]>

functional.desy: remove fft namings

120369d

Signed-off-by: David Caro <[email protected]>

pos: adapt to documents

cb5a69b

Signed-off-by: David Caro <[email protected]>

functional.desy: adapt to documents structure

5fb3a5c

Signed-off-by: David Caro <[email protected]>

global: minimal adaptation to documents

1dd708d

Signed-off-by: David Caro <[email protected]>

david-caro force-pushed the hepcrawl_pos_spider branch from c60a4e5 to 1dd708d Compare November 1, 2017 18:27

david-caro merged commit 4f33b74 into inspirehep:master Nov 1, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pos: update pos spider #160

pos: update pos spider #160

spirosdelviniotis commented Aug 10, 2017 •

edited

Loading

david-caro Aug 10, 2017

spirosdelviniotis Aug 11, 2017

david-caro Aug 10, 2017

david-caro Aug 10, 2017

david-caro Aug 10, 2017

spirosdelviniotis Aug 11, 2017

david-caro Aug 21, 2017

david-caro Aug 21, 2017

david-caro Aug 22, 2017

david-caro Aug 22, 2017

david-caro Aug 22, 2017

spirosdelviniotis commented Aug 22, 2017

		@@ -35,8 +41,13 @@ def scrape_pos_page_body():


		@pytest.fixture

pos: update pos spider #160

pos: update pos spider #160

Conversation

spirosdelviniotis commented Aug 10, 2017 • edited Loading

Description

Related Issue

Motivation and Context

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spirosdelviniotis commented Aug 22, 2017

spirosdelviniotis commented Aug 10, 2017 •

edited

Loading