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 unique_filename not depend on repr. #560

Merged
merged 3 commits into from Aug 1, 2019

Conversation

euresti
Copy link
Contributor

@euresti euresti commented Jul 30, 2019

For debugging purposes, when a method is created it needs to be put
into a "filename". This filename was generated using repr(attrs)
however this meant that the filename could change every time the
program was loaded depending on the whims of repr. (In particular
free functions and lambdas would include ids)

This solves the problem by changing the name to be <attrs generated
{method} {module}.{qualname}> If it should happen that the name is
taken then -2 will be appended and incremented until a free filename
is found.

Fixes #558

I'll wait on a reply as to whether this is more or less what we were looking for before I go through the Pull Request checklist.

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.

  • Added tests for changed code.
  • New features have been added to our Hypothesis testing strategy.
  • Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
    • ...and used in the stub test file tests/typing_example.py.
  • Updated documentation for changed code.
    • New functions/classes have to be added to docs/api.rst by hand.
    • Changes to the signature of @attr.s() have to be added by hand too.
    • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
  • Documentation in .rst files is written using semantic newlines.
  • Changes (and possible deprecations) have news fragments in changelog.d.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

For debugging purposes, when a method is created it needs to be put
into a "filename".  This filename was generated using `repr(attrs)`
however this meant that the filename could change every time the
program was loaded depending on the whims of repr.  (In particular
free functions and lambdas would include ids)

This solves the problem by changing the name to be <attrs generated
{method} {module}.{qualname}> If it should happen that the name is
taken then -2 will be appended and incremented until a free filename
is found.
@euresti euresti requested a review from hynek July 30, 2019 14:12
@euresti
Copy link
Contributor Author

euresti commented Jul 30, 2019

I just tested on some code and this is what the traceback looks like. This is a file with 2 classes called Foo just to show the numbering scheme.

Traceback (most recent call last):
  File "foo.py", line 24, in <module>
    Foo()
  File "<attrs generated init __main__.Foo-2>", line 2, in __init__
    self.__attrs_post_init__()
  File "foo.py", line 20, in __attrs_post_init__
    raise Exception("Foo")
Exception: Foo

Traceback (most recent call last):
  File "foo.py", line 30, in <module>
    X(6)
  File "<attrs generated init __main__.Foo>", line 3, in __init__
    self.__attrs_post_init__()
  File "foo.py", line 12, in __attrs_post_init__
    raise Exception("Foo")
Exception: Foo

@hynek
Copy link
Member

hynek commented Jul 30, 2019

Would you mind adding a test for that (substring check is fine)?

And a newsfragment please. 😇

@euresti
Copy link
Contributor Author

euresti commented Jul 31, 2019

All right. Let me know if something else is needed.

Copy link

@wsanchez wsanchez left a comment

Choose a reason for hiding this comment

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

Awesome

@hynek
Copy link
Member

hynek commented Aug 1, 2019

Thanks you so much David, this looks great!

@hynek hynek merged commit eda9f2d into python-attrs:master Aug 1, 2019
@euresti euresti deleted the unique_filename branch February 18, 2021 17:09
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.

init unique_filename is problematic for exception aggregation
3 participants