reflex icon indicating copy to clipboard operation
reflex copied to clipboard

Refactored project dependencies 'plotly' and 'redis'

Open palashcode opened this issue 2 years ago β€’ 3 comments

All Submissions:

  • [x] Have you followed the guidelines stated in CONTRIBUTING.md file?
  • [x] Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

New Feature Submission:

  • [x] Does your submission pass the tests?
  • [x] Have you linted your code locally prior to submission?

Changes To Core Features:

  • [x] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [ ] Have you written new tests for your core changes, as applicable?
  • [x] Have you successfully ran tests with your changes locally?

After these steps, you're ready to open a pull request.

Refactored project dependencies:

  • Moved 'plotly' and 'redis' dependencies from regular dependencies to extra dependencies using Poetry. This change ensures that these dependencies are only installed when required, reducing the overall project dependencies.
  • Added try-except to capture the error when the user needs the package and it is required to be installed.
  • To install these extra dependencies when required, you can use the following command: poetry install -E plotly -E redis

closes #1240

palashcode avatar Jul 01 '23 17:07 palashcode

This change assume that all users will use poetry, which may not be the case. We are using it for the project itself, but we shouldn't force it on the end users of the package.

Lendemor avatar Jul 01 '23 17:07 Lendemor

Do you mean error should be this

raise ImportError(
            "plotly is not installed\n"
        ) from e

Instead of this

raise ImportError(
            "plotly is not installed\n" "install it using 'poetry install -E plotly'"
        ) from e

And then user can install it using pip or something else also.

palashcode avatar Jul 02 '23 04:07 palashcode

Part of my concern with this change is that it increases friction for new users and might be breaking workflows for existing users. Although i do agree generally about trying to reduce the footprint of reflex, particularly for advanced use cases, we want to keep the pip install reflex step very simple.

Because extras are always extra, there's no python packaging mechanism that I'm aware of to have the base package (reflex) be the complete installation, other than creating a second (reflex-core) package and then just treating the reflex name as a metapackage that installs all of the reflex-core with all extras, and allowing customization of extras for reflex-core in advanced use case.

I expect to put some more time into this problem and PR tomorrow, thanks for your patience and contribution.

masenf avatar Jul 06 '23 00:07 masenf

Close in favor of #1295

masenf avatar Jul 18 '23 19:07 masenf