Open moznuy opened 2 years ago
Any input will be appreciated
Are there any updates on this? #233 has been merged, which means that aioredis
can be removed here as well.
As for the outstanding todo tasks on this PR it seems that they mostly are referring to dev dependencies (questions related to tox, pytest and even sorting dependencies).
I'd love to see at least the removal of aioredis
released soon as it's not a dependency that is used anymore and I'd hate to see it installed when using this package when it's not used anywhere.
Not sure if I could contribute in any way to help get this in, but as for the questions I could provide some info for:
Remove types-six? (six is removed)
I believe this makes sense to remove, seeing as six
isn't being used in the package anymore.
Is pytest-xdist needed?
This package provides concurrency to tests being run, which is used in both test
and test_oss
make commands. pytest -n auto
tells pytest ot run with 'auto' number of workers (count depends on system running the tests).
Is tox(with tox-env) needed?
It doesn't look like tox
is being used? CI runs on Github Actions with a matrix over different Python versions, though it might still be useful to run manually before it hits CI. It's not documented anywhere though.
Poetry 1.2+ (IIRC) introduces dependency groups instead of just main + dev, which might make sense to utilise for optional local development stuff like this. Or at least have some way of just optionally installing these local development tools. I guess dev dependencies can be marked as optional, just as main dependencies can be?
Are there any updates on this? https://github.com/redis/redis-om-python/pull/233 has been merged, which means that aioredis can be removed here as well.
The problem was/is that the communication between maintainers and contributors was non existent (maybe something has changed). I would glad to come back and rebase onto HEAD / finish PR, if there is a chance this might be merged.
This package provides concurrency to tests being run, which is used in both test and test_oss make commands. pytest -n auto tells pytest ot run with 'auto' number of workers (count depends on system running the tests).
As far as I remember, it was faster for me to run tests without concurrency than with it. The question is "is it really needed?", as stated in the #234.
As far as I remember, it was faster for me to run tests without concurrency than with it. The question is "is it really needed?", as stated in the https://github.com/redis/redis-om-python/issues/234.
Fair enough! Hadn't noticed that point in #234 - the test suite seems to be fairly quick already, just a minute for the suite if looking at the CI run, so personally I don't really see the use of xdist.. especially if it's slower in practice! :smile:
The problem was/is that the communication between maintainers and contributors was non existent (maybe something has changed). I would glad to come back and rebase onto HEAD / finish PR, if there is a chance this might be merged.
I was wondering about this yesterday, if there was any public place for discussion outside of just GitHub issues, but couldn't find any at a cursory glance. It's definitely frustrating to have things in limbo, so hopefully communication will improve :pray:
Fixes #234
Changes
hiredis
,email-validator
optional for public and required for dev dependencies.hiredis
,email-validator
to package extras.pptree
,cleo
,six
from public dependencies.ipdb
,pylint
from dev dependencies.types-redis
,types-six
to dev dependencies.six
removal.make lint format test
.poetry update
?types-six
? (six is removed)pytest-xdist
needed?tox
(withtox-env
) needed?Comparison of dist/<>/setup.py:
Before
After