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

[BUG] Plans to update the build frontend? #491

Open
3 tasks done
skandermoalla opened this issue Jul 19, 2023 · 2 comments
Open
3 tasks done

[BUG] Plans to update the build frontend? #491

skandermoalla opened this issue Jul 19, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@skandermoalla
Copy link
Contributor

skandermoalla commented Jul 19, 2023

Describe the bug

I'm following the CONTRIBUTING.md guide and installing the package with python setup.py develop yields lots of deprecation warnings:

        ********************************************************************************
        The license_file parameter is deprecated, use license_files instead.

        By 2023-Oct-30, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.

        See https://setuptools.pypa.io/en/latest/userguide/declarative_config.html for details.
        ********************************************************************************

        ********************************************************************************
        Please avoid running ``setup.py`` and ``easy_install``.
        Instead, use pypa/build, pypa/installer or other
        standards-based tools.

        See https://github.com/pypa/setuptools/issues/917 for details.
        ********************************************************************************

        ********************************************************************************
        Please avoid running ``setup.py`` directly.
        Instead, use pypa/build, pypa/installer or other
        standards-based tools.

        See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
        ********************************************************************************

I think TensorDict still relies on setuptools as a build frontend (i.e. calling setup.py directly) for most of its build routines.
It's been known for a while (even before TensorDict was started?) that setuptools was deprecated as a build frontend, but it seems to still be used in many packages (I think PyTorch as well, and probably how TensorDict and TorchRL picked it up).
This has already created some issues before and seems like it's going to create many more as the deprecation continues.
Why you shouldn't invoke setup.py directly is a good intro to the problem.

Are there any plans to update the build routines and move towards the new way of using setuptools (i.e. as a backend only)? https://setuptools.pypa.io/en/latest/build_meta.html#build-system-support
I think that since TensorDict (and TorchRL) are still under very quick development, it would be a valuable move to make the upgrade soon before it becomes too complicated.

Checklist

  • I have checked that there is no similar issue in the repo (required)
  • I have read the documentation (required)
  • I have provided a minimal working example to reproduce the bug (required)
@skandermoalla skandermoalla added the bug Something isn't working label Jul 19, 2023
@vmoens
Copy link
Contributor

vmoens commented Jul 19, 2023

Thanks for raising this!
I agree it's not ideal, as you mentioned it's something we picked from pytorch and other domain libs.
I can investigate what's the plan wrt this within pytorch and keep you updated here.
Small note though: I'm more open to change things on the RL side than tensordict, the reason being that parts of this lib is at risk of being moved and merged in core, so until we have a clear plan for that I don't think I will spend time refactoring the setup of this lib. Hope that makes sense!

@skandermoalla
Copy link
Contributor Author

Small note though: I'm more open to change things on the RL side than tensordict, the reason being that parts of this lib is at risk of being moved and merged in core, so until we have a clear plan for that I don't think I will spend time refactoring the setup of this lib. Hope that makes sense!

Makes sense! I could have raised the issue in TorchRL as well. I'd be happy to help with the refactor!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants