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.

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

Added recipe for linecache2 #887

Merged
merged 8 commits into from
Aug 16, 2016
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions recipes/linecache2/inspect_fodder2.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--- linecache2/tests/inspect_fodder2.py
+++ linecache2/tests/inspect_fodder2.py
@@ -99,16 +99,16 @@
method_in_dynamic_class = f().g

#line 101
-def keyworded(*arg1, arg2=1):
- pass
+# def keyworded(*arg1, arg2=1):
+# pass

#line 105
-def annotated(arg1: list):
- pass
+# def annotated(arg1: list):
+# pass

#line 109
-def keyword_only_arg(*, arg):
- pass
+# def keyword_only_arg(*, arg):
+# pass

from functools import wraps
45 changes: 45 additions & 0 deletions recipes/linecache2/meta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{%set name = "linecache2" %}
{%set version = "1.0.0" %}
{%set hash_type = "sha256" %}
{%set hash_val = "4b26ff4e7110db76eeb6f5a7b64a82623839d595c2038eeda662f2a2db78e97c" %}

package:
name: {{ name }}
version: {{ version }}

source:
fn: {{ name }}-{{ version }}.tar.gz
url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
{{ hash_type }}: {{ hash_val }}
patches:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the least we will want to xref (as a comment) these patches with PRs on linecache2. I do have some concerns that we are adding py2k support to a project to a codebase that isn't py2k compatible. Is that the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a line note, but you can see a discussion of the issue in the original repo here - it isn't considered something worth patching in the original because there's no problem installing with pip.

I do have some concerns that we are adding py2k support to a project to a codebase that isn't py2k compatible. Is that the case?

This is definitely supposed to be py2k - it's a backport of linecache for earlier versions of python.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me it looks like we may be extending their test suite. Is that not the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My patches aren't extending the suite - I'm explicitly commenting out the tests that caused build issues when byte-compiling for python 2. As such, I consider this more of a reduction in function than an expansion.

- inspect_fodder2.patch # [not py3k]
- test_linecache.patch # [not py3k]
# inspect_fodder2 kills the build when byte-compiling on py 2.x
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is one of those things that would not happen if we were using pip to install it. This is the 3rd package in conda-forge that needs this workaround... Maybe it is time to re-visit #528

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we send these upstream or at least raise an issue? Would be great if they make some adjustments for these issues.

# the patch removes inspect_fodder2 and refs to it

build:
number: 0
script: python setup.py install --single-version-externally-managed --record=record.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pmlandwehr can we experiment here? Change the script line to pip install --no-deps . and remove the patches. Let's see if the install works without it.



requirements:
build:
- python
- setuptools
- pbr

run:
- python

test:
imports:
- {{ name }}

about:
home: https://github.com/testing-cabal/linecache2
license: PSF 1.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to double check on the version ( testing-cabal/linecache2#10 (comment) ).

summary: 'Backport of the linecache module'

extra:
recipe-maintainers:
- pmlandwehr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any interest in being a maintainer, @anguslees? Also any feedback on this PR?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm fine with being added as a maintainer - or not, if anyone has some objection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

109 changes: 109 additions & 0 deletions recipes/linecache2/test_linecache.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
--- linecache2/tests/test_linecache.py
+++ linecache2/tests/test_linecache.py
@@ -13,8 +13,6 @@
NONEXISTENT_FILENAME = FILENAME + '.missing'
INVALID_NAME = '!@$)(!@#_1'
EMPTY = ''
-TESTS = 'inspect_fodder inspect_fodder2 mapping_tests'
-TESTS = TESTS.split()
TEST_PATH = os.path.dirname(__file__)
MODULES = "linecache abc".split()
MODULE_PATH = os.path.dirname(FILENAME)
@@ -37,7 +35,67 @@

SOURCE_3 = '''
def f():
- return 3''' # No ending newline
+ return 3''' # No ending newline
+
+
+class TempFile:
+
+ def setUp(self):
+ super().setUp()
+ with tempfile.NamedTemporaryFile(delete=False) as fp:
+ self.file_name = fp.name
+ fp.write(self.file_byte_string)
+ self.addCleanup(support.unlink, self.file_name)
+
+
+class GetLineTestsGoodData(TempFile):
+ # file_list = ['list\n', 'of\n', 'good\n', 'strings\n']
+
+ def setUp(self):
+ self.file_byte_string = ''.join(self.file_list).encode('utf-8')
+ super().setUp()
+
+ def test_getline(self):
+ with open(self.file_name) as fp:
+ for index, line in enumerate(fp):
+ if not line.endswith('\n'):
+ line += '\n'
+
+ cached_line = linecache.getline(self.file_name, index + 1)
+ self.assertEqual(line, cached_line)
+
+ def test_getlines(self):
+ lines = linecache.getlines(self.file_name)
+ self.assertEqual(lines, self.file_list)
+
+
+class GetLineTestsBadData(TempFile):
+ # file_byte_string = b'Bad data goes here'
+
+ def test_getline(self):
+ self.assertRaises((SyntaxError, UnicodeDecodeError),
+ linecache.getline, self.file_name, 1)
+
+ def test_getlines(self):
+ self.assertRaises((SyntaxError, UnicodeDecodeError),
+ linecache.getlines, self.file_name)
+
+
+class EmptyFile(GetLineTestsGoodData, unittest.TestCase):
+ file_list = []
+
+
+class SingleEmptyLine(GetLineTestsGoodData, unittest.TestCase):
+ file_list = ['\n']
+
+
+class GoodUnicode(GetLineTestsGoodData, unittest.TestCase):
+ file_list = ['á\n', 'b\n', 'abcdef\n', 'ááááá\n']
+
+
+class BadUnicode(GetLineTestsBadData, unittest.TestCase):
+ file_byte_string = b'\x80abc'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we adding new tests for them? Thoughts on this @pelson?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this stuff though, @pmlandwehr? Looks like an addition to me. 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, whoops. Yeah - that's taken from the patch mentioned in this comment That patch was applied to a different commit than the current one in the base repo, so I ported those changes into a new file. That could hasn't been thoroughly reviewed; rather, I was trusting that the various adaptations were indeed necessary for providing tests missing from the original. The fact that the original patches weren't sufficient surprised me, and resulted in the commenting spree in test_linecache.py

So: before passing this on to upstream we should establish that everything in theinspect_fodder.py patch is still essential to the build.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems we forgot to delete the patch after we stopped using it. Proposing removal in PR ( conda-forge/linecache2-feedstock#2 ).

+


class LineCacheTests(unittest.TestCase):
@@ -61,13 +119,6 @@
self.assertEqual(getline(EMPTY, 1), EMPTY)
self.assertEqual(getline(INVALID_NAME, 1), EMPTY)

- # Check whether lines correspond to those from file iteration
- for entry in TESTS:
- filename = os.path.join(TEST_PATH, entry) + '.py'
- with open(filename) as file:
- for index, line in enumerate(file):
- self.assertEqual(line, getline(filename, index + 1))
-
# Check module loading
for entry in MODULES:
filename = os.path.join(MODULE_PATH, entry) + '.py'
@@ -90,12 +141,13 @@

def test_clearcache(self):
cached = []
- for entry in TESTS:
- filename = os.path.join(TEST_PATH, entry) + '.py'
+ for entry in MODULES:
+ filename = os.path.join(MODULE_PATH, entry) + '.py'
cached.append(filename)
linecache.getline(filename, 1)

# Are all files cached?
+ self.assertNotEqual(cached, [])
cached_empty = [fn for fn in cached if fn not in linecache.cache]
self.assertEqual(cached_empty, [])