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

Remove use of attrs #3381

Closed
ionelmc opened this issue Apr 10, 2018 · 12 comments
Closed

Remove use of attrs #3381

ionelmc opened this issue Apr 10, 2018 · 12 comments
Labels
type: question general question, might be closed after 2 weeks of inactivity

Comments

@ionelmc
Copy link
Member

ionelmc commented Apr 10, 2018

So I'm having an issue with this 07b2b18

My project is stuck in a little dependency hell caused by attrs and it's lack of stability. One release removes some stuff, one month later another release adds some other stuff and so on. Some package depends on removed behavior (like accessing attributes via class), pytest depends on new features (converters or something).

Can we revert 07b2b18 - I really don't see what it improves.

@pytestbot
Copy link
Contributor

GitMate.io thinks possibly related issues are #3080 (Remove pytest_namespace), #1229 (Remove plugins_index from docs?), #3083 (Remove metafunc.addcall), #3079 (Remove yield tests), and #3379 (remove markinfo and their apis).

@pytestbot pytestbot added the type: enhancement new feature or API change, should be merged into features branch label Apr 10, 2018
@blueyed
Copy link
Contributor

blueyed commented Apr 10, 2018

I think the plan is to use attrs more over time.

@ionelmc
Copy link
Member Author

ionelmc commented Apr 10, 2018

So now I ate something and I'm feeling better. I think at the very least pytest should have bold note in the readme: "don't upgrade pytest on an empty stomach".

@RonnyPfannschmidt
Copy link
Member

well - attrs does away with bad things over time - which is infinitely better than trying to keep broken things intact while desperately trying to layer less broken things on top

for stability control we might want to do sometihng about how it is imported and used tho -> but NEVER AGAIN vendoring as is

@nicoddemus
Copy link
Member

It is unfortunate that the API has not been as solid as we expected, the problem is that we are using more and more attrs (specially in features) so removing it is not something trivial to do at this point.

API instability and dependency issues with other libraries is something that we should watch closely and perhaps decide to take some action in the future if this becomes a bigger problem, but I think it is early to say that attrs API is unstable and not suitable to be used in an upstream tool like pytest.

Having said all this, @ionelmc can you describe which versions are in conflict and if you solved it somehow, how you did it?

@nicoddemus nicoddemus added type: question general question, might be closed after 2 weeks of inactivity type: infrastructure improvement to development/releases/CI structure and removed type: enhancement new feature or API change, should be merged into features branch labels Apr 10, 2018
@Drew-Ack
Copy link

@RonnyPfannschmidt , @nicoddemus After looking into attrs, I believe it is good to use in pytest because its use is for only one aspect, class declaration. Although it "obscures" explicit definitions of classes, I believe it makes code more readable, because if a dunder (method) is overridden it shows that it has been overridden for a reason, and isn't boilerplate.

Although I am not anywhere as deep in the code as either of you are, I recognize the value attrs brings and just wanted to voice what I thought of it.

@RonnyPfannschmidt
Copy link
Member

@hynek would you like to comment as well?

@hynek
Copy link
Contributor

hynek commented Apr 14, 2018

I’m not sure what to say. Every dependency inevitably leads to conflicts in Python. We’re not loose with breaking compatibility and if possible, leave a year between deprecation and actual break.

Which totally happened in the mentioned case of accessing Attributes on classes.

Only suggestion I have that pytest probably should delay implementing changes like the convert/converter thing until last minute, so people have the full deprecation cycle to fix their software?

@RonnyPfannschmidt
Copy link
Member

@hynek as always this would be far less troublesome if python didn't have such a limiting module system, i'd like to keep attrs entirely as a implementation detail, but python forces to leak it ^^

@The-Compiler
Copy link
Member

FWIW in qutebrowser, I try to keep compatibility with e.g. Debian stable's system-installed packages, which means attrs 16.3 (but I still want to support the newest version, of course).

There I just use whatever subset of attr features I can without getting into trouble, and find some other solution for the rest (for pytest, that'd probably mean not using converters). It still makes my life a lot easier and there's little I'm missing. Usually it's possible to do (almost) the same thing differently, like in qutebrowser/qutebrowser@9d360f8

@hynek
Copy link
Contributor

hynek commented Apr 15, 2018

To be clear: you can use converters. You just have to live with deprecation warnings. Maybe silence them specifically?

@nicoddemus
Copy link
Member

nicoddemus commented Jul 6, 2018

I believe we can close this?

@nicoddemus nicoddemus removed the type: infrastructure improvement to development/releases/CI structure label Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question general question, might be closed after 2 weeks of inactivity
Projects
None yet
Development

No branches or pull requests

8 participants