Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Drop Python 3.2 support #61

Merged
merged 2 commits into from
Oct 25, 2016
Merged

Drop Python 3.2 support #61

merged 2 commits into from
Oct 25, 2016

Conversation

redapple
Copy link
Contributor

@redapple redapple commented Sep 9, 2016 •
edited
Loading

This started as a fix for Travis-CI builds and py.test>=3.0
See http://doc.pytest.org/en/latest/changelog.html
and pytest-dev/pytest#1627

But we might as well drop python 3.2 support as it's not very commonly used.
See pypa/pip#3796

An alternative is to force py.test<3.0: #62

@codecov-io
Copy link

codecov-io commented Sep 9, 2016 •
edited
Loading

Current coverage is 95.02% (diff: 100%)

Merging #61 into master will not change coverage

@@             master        #61   diff @@
==========================================
  Files             3          3          
  Lines           703        703          
  Methods           0          0          
  Messages          0          0          
  Branches        114        114          
==========================================
  Hits            668        668          
  Misses           20         20          
  Partials         15         15

Powered by Codecov. Last update 4d59c71...f01843d

@redapple redapple changed the title Travis: remove py32 build after py.test>=3.0 dropped support for it Drop Python 3.2 support Sep 9, 2016
@kmike
Copy link
Member

kmike commented Sep 9, 2016

+1 to drop Python 3.2 and Python 2.6 support. What do you think about doing that after a release? Is cssselect ready for 1.0 release now?

@redapple
Copy link
Contributor Author

redapple commented Sep 12, 2016 •
edited
Loading

@kmike , one thing I realized now is that with #60 (and with scrapy/parsel especially), translation of things like div ::text change.

with cssselect 0.9.2

>>> import scrapy
>>> s = scrapy.Selector(text='''<html><body><div>div text<p>p text</p></div></body></html>''')
>>> s.css('div::text')
[<Selector xpath=u'descendant-or-self::div/text()' data=u'div text'>]
>>> s.css('div ::text')
[<Selector xpath=u'descendant-or-self::div/descendant-or-self::text()' data=u'div test'>, <Selector xpath=u'descendant-or-self::div/descendant-or-self::text()' data=u'p text'>]
>>> s.css('div ::text').extract()
[u'div text', u'p text']

with cssselect from master:

>>> import scrapy
>>> s = scrapy.Selector(text='''<html><body><div>div text<p>p text</p></div></body></html>''')
>>> s.css('div::text')
[<Selector xpath=u'descendant-or-self::div/text()' data=u'div text'>]
>>> s.css('div::text').extract()
[u'div text']
>>> s.css('div ::text')
[<Selector xpath=u'descendant-or-self::div/descendant::*/text()' data=u'p text'>]
>>> s.css('div ::text').extract()
[u'p text']

This is scrapy-specific but it may annoy some users.

About the rest, what about #34 ?

@kmike
Copy link
Member

kmike commented Sep 12, 2016

div ::text change looks like a serious backwards compatibility issue. Why is it changed?

@redapple
Copy link
Contributor Author

It's the decendant combinator E F that has changed.
descendant-or-self::E/descendant-or-self::F --> descendant-or-self::E/descendant::F -->
IMO, it's more aligned with what CSS selectors mean.
With the change in #60, parsel tests fail, but cssselect tests are correct.
CSS selectors do not have the equivalent of "descendant-or-self"
div ::text is an extension to cssselect in the end.
But the change may bring more damage than benefits

@redapple
Copy link
Contributor Author

redapple commented Sep 12, 2016 •
edited
Loading

It looks like current div ::text implementation in parsel is also a hack on what cssselect returns (0.9.2):
See https://github.com/scrapy/parsel/blob/master/parsel/csstranslator.py#L25

descendant-or-self::div/descendant-or-self::*/* + "text" pseudo --> descendant-or-self::div/descendant-or-self::text()

@redapple
Copy link
Contributor Author

@kmike , I'll work on reverting the decendant combinator change.
We can work on simplifying/correcting it after a 1.0

@redapple redapple mentioned this pull request Sep 12, 2016
Merged
@redapple
Copy link
Contributor Author

FTR, parsel fails its test suite with #60 https://travis-ci.org/scrapy/parsel/builds/159389010

@redapple redapple merged commit ad27009 into scrapy:master Oct 25, 2016
@hugovk hugovk mentioned this pull request Dec 14, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backward compatibility question
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@redapple @codecov-io @kmike