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] Installing openbb-core also installs ruff, prevents upgrade to ">=0.2.0 #6163

Closed
samueldg opened this issue Mar 4, 2024 · 2 comments · Fixed by #6443
Closed

[Bug] Installing openbb-core also installs ruff, prevents upgrade to ">=0.2.0 #6163

samueldg opened this issue Mar 4, 2024 · 2 comments · Fixed by #6443
Assignees
Labels
bug Fix bug

Comments

@samueldg
Copy link

samueldg commented Mar 4, 2024

Describe the bug

The openbb-core package, required to install openbb, defines ruff as a dependency in the main group, and not the dev group.

Compare:

https://github.com/samueldg/OpenBBTerminal/blob/47541d4c957d7ab366e3b5f07615b4a7a9916c7e/openbb_platform/core/pyproject.toml#L25

with the (correct) top-level pyproject.toml:

https://github.com/samueldg/OpenBBTerminal/blob/47541d4c957d7ab366e3b5f07615b4a7a9916c7e/pyproject.toml#L178

Concretely, this means that you end up with the linter/formatter installed even if you don't want to use it, but more importantly it means that you can't upgrade it to the latest available version (0.3.0) because it clashes with the version rule in the openbb-core dependencies.

To Reproduce

  1. Create the following pyproject.toml in a new directory:

    [tool.poetry]
    name = "openbb-core-test"
    version = "0.1.0"
    description = ""
    authors = ["Test <test@example.com>"]
    
    [tool.poetry.dependencies]
    python = "^3.11"
    openbb-core = "^1.1.2"
    
    [tool.poetry.group.dev.dependencies]
    ruff = "^0.3.0"
    
    [build-system]
    requires = ["poetry-core"]
    build-backend = "poetry.core.masonry.api"
  2. Try to poetry install

  3. Get the following error

    Because openbb-core (1.1.2) depends on ruff (>=0.1.6,<0.2.0)
     and no versions of openbb-core match >1.1.2,<2.0.0, openbb-core (>=1.1.2,<2.0.0) requires ruff (>=0.1.6,<0.2.0).
    So, because openbb-core-test depends on both openbb-core (^1.1.2) and ruff (^0.3.0), version solving failed.

Desktop (please complete the following information):

  • OS: Mac Sonoma 14.3
  • Python version: Tried on 3.11 and 3.10
@samueldg samueldg changed the title [Bug] Installing openbb-core also installs ruff, prevents upgrade to "=0.2.0 [Bug] Installing openbb-core also installs ruff, prevents upgrade to ">=0.2.0 Mar 4, 2024
samueldg added a commit to samueldg/OpenBBTerminal that referenced this issue Mar 4, 2024
Like pytest, ruff isn't required to run the code, only in development.

(Similar to the main `pyproject.toml`, which has `ruff` in the same dependency group.)

Closes OpenBB-finance#6163
@deeleeramone
Copy link
Contributor

deeleeramone commented Mar 4, 2024

Hi there,

Ruff and Black are both used in the PackageBuilder class, which is responsible for creating the static assets required to run the Python interface.

https://github.com/OpenBB-finance/OpenBBTerminal/blob/develop/openbb_platform/core/openbb_core/app/static/package_builder.py

These assets are built every time an extension is installed or removed.

There should be nothing preventing you from installing a newer version of Ruff independently, just ignore the pip warning. This isn't really a bug though, so this should be a Feature Request for updating the version of Ruff.

@samueldg
Copy link
Author

samueldg commented Mar 5, 2024

Thanks @deeleeramone for the quick response!

I closed #6164 because the fix doesn't work as is. I was unaware ruff was being used in conjunction with code generation.

There should be nothing preventing you from installing a newer version of Ruff independently, just ignore the pip warning

If you use poetry this isn't a warning, it's straight up a dependency resolution error. You can go and install ruff manually after, but knowing that openbb uses it at runtime means you'd likely break that functionality.

this should be a Feature Request for updating the version of Ruff.

Upgrading is definitely nicer than staying behind, but my concern was more that having it as a runtime dependency in the first place constrains the dev dependencies for downstream repos (e.g. if you upgrade, you also force repos to upgrade othewise they'll have a similar conflict).


As for closing this issue, I think it's still relevant because I suspect other people will stumble into this issue, especially as ruff gains popularity, and recently increased the minor version which means the latest releases will not be compatible with what's declared here.

A couple alternative solutions for this issue:

  • Run the linters if they're available on the path, similar to how Django runs black on generated code.
  • Use a more permissive version range for ruff (although this means you're more likely to have to deal with compatibility issues.
  • Generate the code already linted/formatted (this comment seems to suggest linting was a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug
Projects
None yet
3 participants