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

Update setup.py syntax for pipenv (bug) compliancy #394

Merged
merged 4 commits into from
Mar 7, 2018
Merged

Update setup.py syntax for pipenv (bug) compliancy #394

merged 4 commits into from
Mar 7, 2018

Conversation

apiraino
Copy link
Contributor

@apiraino apiraino commented Mar 1, 2018

Hi,

I'm using your library (cool, thanks!) and I'm migrating my project to using pipenv instead of pip.

Pipenv has a bug that prevents to correctly parse the old extra_requires syntax. Apparently this syntax is old and superseded by install_requires.

In short, what happens is that in my py3 virtualenv, pipenv wrongly installs functools32 and that breaks the whole package installation process.

This patch allows me to use jsonschema in my project migrated to pipenv.

I'd be happy to have your opinion on this small patch, thanks!

setup.py Outdated
@@ -24,8 +24,7 @@
]

extras_require = {
"format": ["rfc3987", "strict-rfc3339", "webcolors"],
":python_version=='2.7'": ["functools32"],
"format": ["rfc3987", "strict-rfc3339", "webcolors"]
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 was wondering if we can move everything out of extra_require but I could not find in any documentation the proper syntax for this format key.

Any hint?

Copy link

Choose a reason for hiding this comment

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

Don’t think so. The format key is using extras_require as it is originally designed to do, i.e. support the package[extra] installation syntax. There are no alternatives to this feature. extras_require itself is not obsolete, only using it for environment markers is.

@Julian
Copy link
Member

Julian commented Mar 2, 2018

Hey!

Looks like the build is failing, could you have a look to see if that's relevant?

Otherwise seems reasonable thanks!

@apiraino
Copy link
Contributor Author

apiraino commented Mar 2, 2018

Hi Julian, the build is failing in the doc generation, I'm not sure how to fix a.t.m.

EDIT: these tests are also failing to me on branch master. I must be missing some tooling.

  pypy-build: commands succeeded
  pypy-tests: commands succeeded
ERROR:   docs-html: commands failed
  docs-doctest: commands succeeded
ERROR:   docs-linkcheck: commands failed
ERROR:   docs-spelling: commands failed
  docs-style: commands succeeded
ERROR:   style: commands failed

I can have a look at it, though

Stacktrace:

Traceback (most recent call last):
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/cmdline.py", line 304, in main
    app.build(args.force_all, filenames)
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/application.py", line 325, in build
    self.builder.build_all()
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/builders/__init__.py", line 290, in build_all
    self.build(None, summary='all source files', method='all')
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/builders/__init__.py", line 394, in build
    self.write(docnames, list(updated_docnames), method)
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/builders/__init__.py", line 431, in write
    self._write_serial(sorted(docnames))
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/builders/__init__.py", line 440, in _write_serial
    self.write_doc(docname, doctree)
  File "/opt/python/pypy2.7-5.8.0/lib-python/2.7/contextlib.py", line 24, in __exit__
    self.gen.next()
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/util/logging.py", line 240, in pending_warnings
    memhandler.flushTo(logger)
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/util/logging.py", line 206, in flushTo
    logger.handle(record)
  File "/opt/python/pypy2.7-5.8.0/lib-python/2.7/logging/__init__.py", line 1309, in handle
    self.callHandlers(record)
  File "/opt/python/pypy2.7-5.8.0/lib-python/2.7/logging/__init__.py", line 1349, in callHandlers
    hdlr.handle(record)
  File "/opt/python/pypy2.7-5.8.0/lib-python/2.7/logging/__init__.py", line 768, in handle
    rv = self.filter(record)
  File "/opt/python/pypy2.7-5.8.0/lib-python/2.7/logging/__init__.py", line 630, in filter
    if not f.filter(record):
  File "/home/travis/build/Julian/jsonschema/.tox/docs-spelling/site-packages/sphinx/util/logging.py", line 380, in filter
    raise SphinxWarning(location + ":" + message)
SphinxWarning: /home/travis/build/Julian/jsonschema/jsonschema/validators.py:docstring of jsonschema.validators.validates::py:class reference target not found: callable
Warning, treated as error:
/home/travis/build/Julian/jsonschema/jsonschema/validators.py:docstring of jsonschema.validators.validates::py:class reference target not found: callable
make: *** [spelling] Error 2
ERROR: InvocationError: '/usr/bin/make -f /home/travis/build/Julian/jsonschema/docs/Makefile BUILDDIR=/home/travis/build/Julian/jsonschema/.tox/docs-spelling/tmp/build SPHINXOPTS=-a -c /home/travis/build/Julian/jsonschema/docs/ -n -T -W  spelling'

@apiraino
Copy link
Contributor Author

apiraino commented Mar 3, 2018

ok @Julian I've had some playtime digging the issue here.

First I had to install a couple of packages to reproduce the TravisCI results. Then after some trial-and-error I've discovered that the docs-* builds fail because the documentation is not compliant to the standards required by running sphinx-build -n (i.e. nitpicky-mode).

IMO these failures have nothing to do with this patch (aside this: 8ae6e6f).

If you wish, I can prepare another small pr to update the documentation (clearing up package requirements for tox tests) and "calm down" the sphinx nitpicking:

diff --git a/docs/conf.py b/docs/conf.py
index c7126f2..1d65864 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -253,3 +253,9 @@ texinfo_documents = [
 # -- Options for sphinxcontrib-spelling -----------------------------------
 
 spelling_word_list_filename = "spelling-wordlist.txt"
+
+nitpick_ignore = [
+    ('py:class', 'callable'),
+    ('py:class', 'functools.lru_cache'),
+]

with this patch, all the tests pass (tested locally, not committed yet).

Let me know your thoughts on the matter :)

@Julian
Copy link
Member

Julian commented Mar 5, 2018

Hey! Thanks so much for having a look.

That fix strikes me like hiding the problem rather than a fix though -- sphinx should have no problem finding those two objects, so something is up... And yeah nitpick mode is basically the only way to get Sphinx to do normal things, otherwise it's way too lenient about errors and will render total nonsense into its output without anyone noticing...

But it definitely doesn't look related to this change, so if you don't see anything else I'm happy to merge and jump down the rabbit hole myself.

Thanks again!

It definitely does not

@Julian Julian merged commit d1392e4 into python-jsonschema:master Mar 7, 2018
Julian added a commit that referenced this pull request Mar 7, 2018
See #394 (comment)
which goes away when downgrading.

I've also just had the fun manually verifying the objects.inv
for both of those versions correctly contain the object
we reference.

Hooray...
@Julian
Copy link
Member

Julian commented Mar 7, 2018

OK, pretty sure I've tracked this down to some sort of Sphinx 1.7 bug, but I have no more energy to find it specifically.

Merged though! Thanks for the PR!

@apiraino
Copy link
Contributor Author

apiraino commented Mar 7, 2018

You beat me for one second. After sweat and tears and a lot of trial-and-error I reached the same results :-)

Yes, you need to pin sphinx==1.6.7 in your tests requirement.txt, that will fix everything (tested locally and it works).

I am pushing to a test repo to submit a POC to the Sphinx maintainers.

@Julian
Copy link
Member

Julian commented Mar 7, 2018

Hah awesome, nice work! Clearly more energy than I have :)

Julian added a commit that referenced this pull request Aug 13, 2020
86f52b87 Fix a clear copy-paste error in the case names for tests from #394.
ec18a7d0 Merge pull request #360 from notEthan/duplicate_uris
cd9a4b9d change schemas with duplicate URIs http://localhost:1234/folder/
43e190e0 Merge pull request #394 from rjmill/rjmill/bools-and-1s-and-0s-inside-objects-and-arrays
85f0d459 Merge pull request #419 from ChALkeR/chalker/format-uri
54436216 Merge pull request #420 from ChALkeR/chalker/format/ip6
ad47b726 Add long valid and invalid ipv6
b2ab70ec More optional ipv6 tests
37189ae6 Test that uri format scheme is validated for allowed chars
2106ed17 backport uniqueItems cases to draft3
49fc2a95 backport const test cases to draft6
79fe60c0 more tests for true != 1 and false != 0 inside objects/arrays

git-subtree-dir: json
git-subtree-split: 86f52b87e3d572b8f808dd074a6b95c3695ba992
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants