Skip to content

Don't use deprecated methods in resolve_with() #95

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

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

handrews
Copy link
Contributor

Fixes #86: resolve_with(), which is not deprecated, was using the deprecated is_valid() method. Replaced with a lazily instantiated validator object.

I'm happy to change this approach, it's just what first came to mind. The linter was unhappy with the complexity of resolve_with() unless I moved the self._base_uri_validator instantiation check to its own method.

End of the tox output (run with 3.7.16, 3.8.13, 3.9.6, 3.10.10, 3.11.2, 3.12.0a5), run with this branch on top of the branch for PR #94 to avoid superfluous reformatting output:

---------- coverage: platform darwin, python 3.12.0-alpha-5 ----------
Name                                                             Stmts   Miss  Cover
------------------------------------------------------------------------------------
.tox/py312/lib/python3.12/site-packages/rfc3986/__init__.py         16      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/_mixin.py          112      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/abnf_regexp.py      63      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/api.py              15      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/builder.py          66      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/compat.py           10      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/exceptions.py       44      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/iri.py              50      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/misc.py             31      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/normalizers.py      80      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/parseresult.py     167      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/uri.py              30      0   100%
.tox/py312/lib/python3.12/site-packages/rfc3986/validators.py      128      0   100%
------------------------------------------------------------------------------------
TOTAL                                                              812      0   100%

Required test coverage of 100% reached. Total coverage: 100.00%
2848 passed, 4947 warnings in 3.24s
.pkg: _exit> python /Users/handrews/dev/.venv/lib/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
py312: OK ✔ in 4.2 seconds
lint: commands[0]> black -l 78 -t py37 src/rfc3986 tests/
All done! ✨ 🍰 ✨
25 files left unchanged.
lint: commands[1]> flake8 src/rfc3986
  py37: OK (4.24=setup[1.01]+cmd[3.23] seconds)
  py38: OK (4.06=setup[0.67]+cmd[3.39] seconds)
  py39: OK (4.72=setup[1.44]+cmd[3.28] seconds)
  py310: OK (3.94=setup[0.70]+cmd[3.24] seconds)
  py311: OK (4.13=setup[0.79]+cmd[3.34] seconds)
  py312: OK (4.20=setup[0.73]+cmd[3.47] seconds)
  lint: OK (0.73=setup[0.01]+cmd[0.26,0.47] seconds)
  congratulations :) (26.14 seconds)

Note that _mixin.py was previously 112 statements, now 120.

resolve_with(), which is not deprecated, was using the
deprecated is_valid() method.  Replaced with a lazily instantiated
validator object.
@@ -56,6 +56,15 @@ def authority_info(self):
def _match_subauthority(self):
return misc.SUBAUTHORITY_MATCHER.match(self.authority)

def _get_base_uri_validator(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather this be a property that looks like:

@property 
def _validator(self):
    v = getattr(self, '_cached_validator', None)
    if v is not None:
        return v
    self._cached_validator = validators.Validator().require_presence_of("scheme")
    return self._cached_validator

And then used as

self._validator.validate(base_uri)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sigmavirus24 thanks, I've updated this and added a test needed to cover the case of the validator already existing with this code arrangement.

This required adding a test to cover the line where the validator
has already been created.
@sigmavirus24
Copy link
Collaborator

Thank you @handrews !

@sigmavirus24 sigmavirus24 merged commit 9d847bf into python-hyper:main Feb 22, 2023
@handrews handrews deleted the no-warnings branch February 22, 2023 04:44
@handrews handrews mentioned this pull request Apr 17, 2023
@handrews handrews mentioned this pull request May 19, 2023
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.

URIMixin.resolve_with emits a warning
2 participants