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

resolve weird klein.resource namespace conflict, gently as possible #247

Merged
merged 7 commits into from Jun 28, 2018

Conversation

Projects
None yet
2 participants
@glyph
Member

glyph commented Jun 26, 2018

Fixes #224

Supersedes #225

I think we should probably eventually deprecate klein.resource.KleinResource as the public name and expose it somewhere else, but for the time being, let's resolve things in as friendly a way as possible, so that everything keeps working regardless of what order you import things in.

@glyph glyph requested a review from twisted/twisted-contributors Jun 26, 2018

@glyph

This comment has been minimized.

Show comment
Hide comment
@glyph

glyph Jun 26, 2018

Member

To crib from the original bug description:

>>> import klein.resource
>>> from klein import resource as resource
>>> resource
<special bound method/module klein.resource>
>>> resource()
<klein._resource.KleinResource object at 0x109336b70>
>>> resource.KleinResource
<class 'klein._resource.KleinResource'>

and

>>> from klein import resource as resource
>>> resource
<special bound method/module klein.resource>
>>> resource()
<klein._resource.KleinResource object at 0x10ed26cf8>
>>> resource.KleinResource
<class 'klein._resource.KleinResource'>

It's a tiny bit weird but it's totally consistent and not buggy.

Member

glyph commented Jun 26, 2018

To crib from the original bug description:

>>> import klein.resource
>>> from klein import resource as resource
>>> resource
<special bound method/module klein.resource>
>>> resource()
<klein._resource.KleinResource object at 0x109336b70>
>>> resource.KleinResource
<class 'klein._resource.KleinResource'>

and

>>> from klein import resource as resource
>>> resource
<special bound method/module klein.resource>
>>> resource()
<klein._resource.KleinResource object at 0x10ed26cf8>
>>> resource.KleinResource
<class 'klein._resource.KleinResource'>

It's a tiny bit weird but it's totally consistent and not buggy.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov Jun 26, 2018

Codecov Report

Merging #247 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
+ Coverage   96.59%   96.62%   +0.02%     
==========================================
  Files          34       34              
  Lines        2584     2605      +21     
  Branches      175      175              
==========================================
+ Hits         2496     2517      +21     
  Misses         74       74              
  Partials       14       14
Impacted Files Coverage Δ
src/klein/test/test_resource.py 97.06% <100%> (+0.02%) ⬆️
src/klein/__init__.py 100% <100%> (ø) ⬆️
src/klein/resource.py 100% <100%> (ø) ⬆️
src/klein/test/test_exports.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f17daa...afbf9c7. Read the comment docs.

codecov commented Jun 26, 2018

Codecov Report

Merging #247 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
+ Coverage   96.59%   96.62%   +0.02%     
==========================================
  Files          34       34              
  Lines        2584     2605      +21     
  Branches      175      175              
==========================================
+ Hits         2496     2517      +21     
  Misses         74       74              
  Partials       14       14
Impacted Files Coverage Δ
src/klein/test/test_resource.py 97.06% <100%> (+0.02%) ⬆️
src/klein/__init__.py 100% <100%> (ø) ⬆️
src/klein/resource.py 100% <100%> (ø) ⬆️
src/klein/test/test_exports.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f17daa...afbf9c7. Read the comment docs.

@moshez

moshez approved these changes Jun 28, 2018

Thanks for untangling this!

Possibly make sure that there is less potential confusion by importing resource as _resource in resource.py, and either way this is good to merge.

Show outdated Hide outdated src/klein/resource.py
@glyph

This comment has been minimized.

Show comment
Hide comment
@glyph

glyph Jun 28, 2018

Member

Thanks for the review @moshez !

Member

glyph commented Jun 28, 2018

Thanks for the review @moshez !

@glyph glyph merged commit 8be8f2a into master Jun 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@glyph glyph deleted the resource-superposition branch Jun 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment