Pr ponta grossa

[antoniovendramin]

Ponta Grossa são páginas simples com a data no nome do arquivo PDF.
Legislativo e executivo são no mesmo PDF.

[cuducos]

Hi @antoniovendramin – many thanks for you interest and your PR.

I was studying your code and I made some minor comments that might help you improve the readability of this contribution. I hope they're helpful : )

Many thanks

[cuducos]

I think comprehensions are more readable than map in most cases. For example, this line would be easier to read as:

menor_ano_da_pagina =  min(p['ano'] for p in pdf_infos)

[rennerocha]

Would it be better if the variables are also in english instead of portuguese?

[cuducos]

Is it really need to refine scrape_page? As all parse does is calling scrape_page, the body of scrape_page could be the parse method.

[cuducos]

You started the file using ' to define string, and you changed from here on. Can we have a more consistent use of quotes? I suggest always using ' except when there's the need to use a single quote inside the string.

[cuducos]

Why not extracting this bit (lines 27-40) into a custom method? This method could be a generator (way better than appending to a list).

This would like something like:

    @staticmethod
    def pdfs_info(links):
        link_pattern = '.*/diario-oficial/_?(\d{4})-(\d{2})-(\d{2}).*.pdf'
        for link in links:
            file_name = pdf_link.css('::attr(href)').extract_first()
            link_text = pdf_link.css('::text').extract_first()

            if 'sem_atos' in file_name:
                continue

            info = re.search(link_pattern, file_name)
            year = int(info.group(1))
            if year < self.ano_minimo:
                continue

            month, day = int(info.group(2)), int(info.group(3))
            yield {
                'date': date(year, month, day),
                'is_extra_edition': 'complementar' in link_text
            }

[cuducos]

We could increase readability here splitting the logic and using f-strings, couldn't we?

url_path = response.css('.pager-next a::attr(href)').extract_first()
next_page_url = f'http://www.pontagrossa.pr.gov.br{url_path}'

However @rennerocha might have a better strategy to get absolute URLs from relative URLs in Scrapy ; )

[rennerocha]

If you have a relative URL, you should use urlparse.urljoin instead of doing string manipulation.

As this is something very common to appear during the scraping, Scrapy has this method in the response object:
https://doc.scrapy.org/en/latest/topics/request-response.html#scrapy.http.Response.urljoin

next_page_url = response.urljoin(response.css(".pager-next a::attr(href)").extract_first())

[cuducos]

response.urljoin for the win 🎉

[cuducos]

If we do rename scrape_page as parse I guess we can omit the second argument of Request here because self.parse is the default (but I'm not 100% sure about it, would you mind testing it if that's the case?)

[rennerocha]

self.parse is the default callback for a Request . Get rid of self.scrape_page and put everything in self.parse :-)

[cuducos]

If pdf_info returns a datetime.date (as I suggest above) this could be simplified as:

    gazette_date = pdf_info['date'].strftime('%Y-%m-%d')

(I just avoid using variables called date because if you from datetime import date you end up overwriting the date object).

[cuducos]

All this lines from 50 and on have an extra indentation level. You could delete 4 spaces in the beginning of each of them ; )

[giovanisleite]

Remember to update the cities.md

[antoniovendramin]

Accepted all suggestions.
Thanks!

[Irio]

This municipality not only hasn't selectable text but has a printed pdf inside another.

As in the bottom left of the first page:
http://www.pontagrossa.pr.gov.br/files/diario-oficial/2018-05-01-ed2303.pdf

[Irio]

Thanks for the contribution, @antoniovendramin.