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

Make cattrs and cattr.errors namespaces contain all error types from cattrs.errors #252

Closed
wants to merge 4 commits into from

Conversation

jap
Copy link
Contributor

@jap jap commented Apr 10, 2022

Without this, trying to do something along the lines of

import cattr.errors

try:
  ...
except cattr.errors.BaseValidationError as exc:
  ...

fails miserably because it was only importing StructureHandlerNotFoundError from cattrs.errors

Besides that, importing cattrs (with s) causes cattrs.errors to be imported from cattr.errors which then causes the same problem.

import cattrs.errors also has this issue but Python's import machinery is too complicated for me to understand or explain why.

The only way to get a reachable BaseValidationError is by doing

from cattrs.errors import BaseValidationError

which would cause code review remarks during the day job where we're trying to get rid of "from-imports".

@Tinche
Copy link
Member

Tinche commented Apr 10, 2022

Hi,

cattr is kind of a deprecated namespace, it's not going away but new stuff is only going to be added to cattrs to gently encourage migration. I have very recently moved the actual errors to cattrs.errors, so my first question would be if the main branch still has the problem you're experiencing?

@jap
Copy link
Contributor Author

jap commented Apr 10, 2022

It is, with your main branch checked out:

lil-bob:cattrs spaans$ pip install --editable .
Obtaining file:///Users/spaans/src/cattrs
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... done
Collecting exceptiongroup
  Using cached exceptiongroup-1.0.0rc2-py3-none-any.whl (10 kB)
Collecting attrs>=20
  Using cached attrs-21.4.0-py2.py3-none-any.whl (60 kB)
Building wheels for collected packages: cattrs
  Building editable for cattrs (pyproject.toml) ... done
  Created wheel for cattrs: filename=cattrs-22.2.0.dev0-py3-none-any.whl size=4915 sha256=f4138de6c395a84340443b1d66191eb3fba37cc1e6032639ae4580eeefb5ac6a
  Stored in directory: /private/var/folders/hs/y6mmqk9d4p97tr05d11crdb80000gn/T/pip-ephem-wheel-cache-grv9nq2h/wheels/cb/07/86/7e063245dba45861348d66e20e3ff2da1b3ebd669531de559d
Successfully built cattrs
Installing collected packages: exceptiongroup, attrs, cattrs
Successfully installed attrs-21.4.0 cattrs-22.2.0.dev0 exceptiongroup-1.0.0rc2
lil-bob:cattrs spaans$ python3
Python 3.10.3 (main, Mar 29 2022, 22:28:06) [Clang 13.1.6 (clang-1316.0.21.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import cattrs.errors
>>> dir(cattrs.errors)
['StructureHandlerNotFoundError', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__']

@jap
Copy link
Contributor Author

jap commented Apr 10, 2022

and with my branch:

lil-bob:cattrs spaans$ git checkout origin/error-import 
Previous HEAD position was 9e740a8 Tweak test
HEAD is now at 079b6f7 Ensure all error types are reachable through module imports
lil-bob:cattrs spaans$ python3
Python 3.10.3 (main, Mar 29 2022, 22:28:06) [Clang 13.1.6 (clang-1316.0.21.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import cattr.errors
>>> dir(cattrs.errors)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'cattrs' is not defined
>>> import cattrs.errors
>>> dir(cattrs.errors)
['BaseValidationError', 'ClassValidationError', 'ForbiddenExtraKeysError', 'IterableValidationError', 'StructureHandlerNotFoundError', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__']

@jap
Copy link
Contributor Author

jap commented Apr 10, 2022

I think the from cattr import ..., errors, ... in cattrs.__init__ is to blame, so it may be easier to fix by just letting that import from its own errors module, but then there still is the issue that cattr.errors only imports one class instead of all for people using the legacy namespace.

@jap
Copy link
Contributor Author

jap commented Apr 10, 2022

I put an additional commit in https://github.com/jap/cattrs/tree/error-import2 which takes the above into account, and actually exposes the errors on the cattrs namespace directly, as implied by https://cattrs.readthedocs.io/en/latest/validation.html (which refers to cattrs.ClassValidationError for example, which currently really doesn't exist.)

@Tinche
Copy link
Member

Tinche commented Apr 10, 2022

Sorry, I might have posted my reply too early. Can you try with 792a5f4?

This is what I'm seeing:

$ python -c "import cattrs.errors; print(dir(cattrs.errors))"
['BaseValidationError', 'ClassValidationError', 'ExceptionGroup', 'ForbiddenExtraKeysError', 'IterableValidationError', 'Optional', 'Set', 'StructureHandlerNotFoundError', 'Type', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__']

@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2022

Codecov Report

Merging #252 (be42812) into main (38205da) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head be42812 differs from pull request most recent head e379bce. Consider uploading reports for the commit e379bce to get more accurate results

@@           Coverage Diff           @@
##             main     #252   +/-   ##
=======================================
  Coverage   97.01%   97.01%           
=======================================
  Files          16       16           
  Lines        1339     1340    +1     
=======================================
+ Hits         1299     1300    +1     
  Misses         40       40           
Impacted Files Coverage Δ
src/cattrs/__init__.py 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@jap
Copy link
Contributor Author

jap commented Apr 10, 2022

Confirmed - your latest version fixes the original issue for me.
Note that the cattr.errors namespace is still like it was before, but maybe that is not an issue given that it is deprecated; however, cattrs itself doesn't contain the error classes like the validation page implies.

@Tinche
Copy link
Member

Tinche commented Apr 11, 2022

Agreed, would be great if you could open a new PR (or repurpose this one) to just re-export the exceptions from cattrs.__init__ ;)

@jap
Copy link
Contributor Author

jap commented Apr 12, 2022

I'll make some time for that this week!

@jap jap changed the title Ensure all error types are reachable through module imports Make cattrs and cattr.errors namespaces contain all errors from cattrs.errors Apr 16, 2022
@jap jap changed the title Make cattrs and cattr.errors namespaces contain all errors from cattrs.errors Make cattrs and cattr.errors namespaces contain all error types from cattrs.errors Apr 16, 2022
@jap
Copy link
Contributor Author

jap commented Apr 16, 2022

Rebased against your main branch and made things work.

@Tinche
Copy link
Member

Tinche commented Jul 4, 2022

Sorry, I forgot about this for a while. Can you add a small HISTORY entry and I can merge?

@jap
Copy link
Contributor Author

jap commented Jul 5, 2022

Sure, done!

@Tinche
Copy link
Member

Tinche commented Jul 6, 2022

Thanks, I merged this into main manually manually!

@Tinche Tinche closed this Jul 6, 2022
mergify bot pushed a commit to aws/jsii that referenced this pull request Oct 3, 2022
…3 in /packages/@jsii/python-runtime (#3785)

Updates the requirements on [cattrs](https://github.com/python-attrs/cattrs) to permit the latest version.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/python-attrs/cattrs/blob/main/HISTORY.rst">cattrs's changelog</a>.</em></p>
<blockquote>
<h2>22.2.0 (2022-10-03)</h2>
<ul>
<li><em>Potentially breaking</em>: <code>cattrs.Converter</code> has been renamed to <code>cattrs.BaseConverter</code>, and <code>cattrs.GenConverter</code> to <code>cattrs.Converter</code>.
The <code>GenConverter</code> name is still available for backwards compatibility, but is deprecated.
If you were depending on functionality specific to the old <code>Converter</code>, change your import to <code>from cattrs import BaseConverter</code>.</li>
<li><code>NewTypes &lt;https://docs.python.org/3/library/typing.html#newtype&gt;</code>_ are now supported by the <code>cattrs.Converter</code>.
(<code>[#255](python-attrs/cattrs#255) &lt;https://github.com/python-attrs/cattrs/pull/255&gt;</code><em>, <code>[#94](python-attrs/cattrs#94) &lt;https://github.com/python-attrs/cattrs/issues/94&gt;</code></em>, <code>[#297](python-attrs/cattrs#297) &lt;https://github.com/python-attrs/cattrs/issues/297&gt;</code>_)</li>
<li><code>cattrs.Converter</code> and <code>cattrs.BaseConverter</code> can now copy themselves using the <code>copy</code> method.
(<code>[#284](python-attrs/cattrs#284) &lt;https://github.com/python-attrs/cattrs/pull/284&gt;</code>_)</li>
<li>Python 3.11 support.</li>
<li>cattrs now supports un/structuring <code>kw_only</code> fields on attrs classes into/from dictionaries.
(<code>[#247](python-attrs/cattrs#247) &lt;https://github.com/python-attrs/cattrs/pull/247&gt;</code>_)</li>
<li>PyPy support (and tests, using a minimal Hypothesis profile) restored.
(<code>[#253](python-attrs/cattrs#253) &lt;https://github.com/python-attrs/cattrs/issues/253&gt;</code>_)</li>
<li>Fix propagating the <code>detailed_validation</code> flag to mapping and counter structuring generators.</li>
<li>Fix <code>typing.Set</code> applying too broadly when used with the <code>GenConverter.unstruct_collection_overrides</code> parameter on Python versions below 3.9. Switch to <code>typing.AbstractSet</code> on those versions to restore the old behavior.
(<code>[#264](python-attrs/cattrs#264) &lt;https://github.com/python-attrs/cattrs/issues/264&gt;</code>_)</li>
<li>Uncap the required Python version, to avoid problems detailed in <a href="https://iscinumpy.dev/post/bound-version-constraints/#pinning-the-python-version-is-special">https://iscinumpy.dev/post/bound-version-constraints/#pinning-the-python-version-is-special</a>
(<code>[#275](python-attrs/cattrs#275) &lt;https://github.com/python-attrs/cattrs/issues/275&gt;</code>_)</li>
<li>Fix <code>Converter.register_structure_hook_factory</code> and <code>cattrs.gen.make_dict_unstructure_fn</code> type annotations.
(<code>[#281](python-attrs/cattrs#281) &lt;https://github.com/python-attrs/cattrs/issues/281&gt;</code>_)</li>
<li>Expose all error classes in the <code>cattr.errors</code> namespace. Note that it is deprecated, just use <code>cattrs.errors</code>.
(<code>[#252](python-attrs/cattrs#252) &lt;https://github.com/python-attrs/cattrs/issues/252&gt;</code>_)</li>
<li>Fix generating structuring functions for types with quotes in the name.
(<code>[#291](python-attrs/cattrs#291) &lt;https://github.com/python-attrs/cattrs/issues/291&gt;</code>_ <code>[#277](python-attrs/cattrs#277) &lt;https://github.com/python-attrs/cattrs/issues/277&gt;</code>_)</li>
<li>Fix usage of notes for the final version of <code>PEP 678 &lt;https://peps.python.org/pep-0678/&gt;</code><em>, supported since <code>exceptiongroup&gt;=1.0.0rc4</code>.
(<code>[#303](python-attrs/cattrs#303) &lt;303 &lt;https://github.com/python-attrs/cattrs/pull/303&gt;</code></em>)</li>
</ul>
<h2>22.1.0 (2022-04-03)</h2>
<ul>
<li>cattrs now uses the CalVer versioning convention.</li>
<li>cattrs now has a detailed validation mode, which is enabled by default. Learn more <code>here &lt;https://cattrs.readthedocs.io/en/latest/validation.html&gt;</code>_.
The old behavior can be restored by creating the converter with <code>detailed_validation=False</code>.</li>
<li><code>attrs</code> and dataclass structuring is now ~25% faster.</li>
<li>Fix an issue structuring bare <code>typing.List</code> s on Pythons lower than 3.9.
(<code>[#209](python-attrs/cattrs#209) &lt;https://github.com/python-attrs/cattrs/issues/209&gt;</code>_)</li>
<li>Fix structuring of non-parametrized containers like <code>list/dict/...</code> on Pythons lower than 3.9.
(<code>[#218](python-attrs/cattrs#218) &lt;https://github.com/python-attrs/cattrs/issues/218&gt;</code>_)</li>
<li>Fix structuring bare <code>typing.Tuple</code> on Pythons lower than 3.9.
(<code>[#218](python-attrs/cattrs#218) &lt;https://github.com/python-attrs/cattrs/issues/218&gt;</code>_)</li>
<li>Fix a wrong <code>AttributeError</code> of an missing <code>__parameters__</code> attribute. This could happen
when inheriting certain generic classes – for example <code>typing.*</code> classes are affected.
(<code>[#217](python-attrs/cattrs#217) &lt;https://github.com/python-attrs/cattrs/issues/217&gt;</code>_)</li>
<li>Fix structuring of <code>enum.Enum</code> instances in <code>typing.Literal</code> types.
(<code>[#231](python-attrs/cattrs#231) &lt;https://github.com/python-attrs/cattrs/pull/231&gt;</code>_)</li>
<li>Fix unstructuring all tuples - unannotated, variable-length, homogenous and heterogenous - to <code>list</code>.
(<code>[#226](python-attrs/cattrs#226) &lt;https://github.com/python-attrs/cattrs/issues/226&gt;</code>_)</li>
<li>For <code>forbid_extra_keys</code> raise custom <code>ForbiddenExtraKeyError</code> instead of generic <code>Exception</code>.
(<code>[#225](python-attrs/cattrs#225) &lt;https://github.com/python-attrs/cattrs/pull/225&gt;</code>_)</li>
<li>All preconf converters now support <code>loads</code> and <code>dumps</code> directly. See an example <code>here &lt;https://cattrs.readthedocs.io/en/latest/preconf.html&gt;</code>_.</li>
</ul>

</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/python-attrs/cattrs/commit/405f0291b958ae9eb45ee38febeb91fb65dd644f"><code>405f029</code></a> v22.2.0</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/89de04f57aa774d6abfb0ae62517dc8a8064e3c2"><code>89de04f</code></a> Fix some mor</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/0abbf271461babca203862f3581c20743f6118e0"><code>0abbf27</code></a> Fix tests</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/3b750439aec826a8fd20976ca113f30a27e75408"><code>3b75043</code></a> <strong>notes</strong> is list[str]</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/906b95cfef4903e2d6247abf0fdd72f7da21617a"><code>906b95c</code></a> Reorder HISTORY</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/ed7f86a0ccd9adeab895b9afac2dea69fa02bcff"><code>ed7f86a</code></a> Improve NewTypes (<a href="https://github-redirect.dependabot.com/python-attrs/cattrs/issues/310">#310</a>)</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/e7926599ad44e07d8325ae4072626e1a24705542"><code>e792659</code></a> Fix missing imports of preconf converters (<a href="https://github-redirect.dependabot.com/python-attrs/cattrs/issues/309">#309</a>)</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/e425d6378aa8dfc74bbdd9e152365e1b962fd8cf"><code>e425d63</code></a> Reorder HISTORY</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/cc56b2b873852a4e91b16706a39da00755c87759"><code>cc56b2b</code></a> Remove spurious type comment</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/cbd6f29d2c0ebc40805b3ca0d81accfc330eb10c"><code>cbd6f29</code></a> Reformat</li>
<li>Additional commits viewable in <a href="https://github.com/python-attrs/cattrs/compare/v1.8.0...v22.2.0">compare view</a></li>
</ul>
</details>
<br />


Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>
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