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

Changing packaging version #3764

Merged
merged 8 commits into from
Dec 12, 2022
Merged

Conversation

hjoaquim
Copy link
Contributor

@hjoaquim hjoaquim commented Dec 12, 2022

This PR patch the deprectation of LegacyVersion under packaging - also discussed here: pypa/packaging#631

  • Removes LegacyVersion
  • Specifies that packaging=">=22.0"

Steps taken:

poetry add packaging=">=22.0"
poetry export -f requirements.txt  -o requirements.txt --without-hashes --dev
poetry export -f requirements.txt  -o requirements-full.txt --extras prediction --extras optimization --without-hashes --dev

Fix:

@hjoaquim hjoaquim added the bug Fix bug label Dec 12, 2022
@hjoaquim hjoaquim self-assigned this Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@jmaslek
Copy link
Collaborator

jmaslek commented Dec 12, 2022

I think the better solution will be to change:

current_version: Union[version.LegacyVersion, version.Version]

to

current_version: version.Version

And then add packaging^22.

Thoughts?

@hjoaquim hjoaquim added the do not merge Label to prevent pull request merge label Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@hjoaquim
Copy link
Contributor Author

@jmaslek agreed and changed

@hjoaquim hjoaquim removed the do not merge Label to prevent pull request merge label Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@hjoaquim
Copy link
Contributor Author

Does the Tests / General Linting create a fresh environment each time? The error does not make sense since version.parse() returns only Version with packaging 22

@jmaslek
Copy link
Collaborator

jmaslek commented Dec 12, 2022

Does the Tests / General Linting create a fresh environment each time? The error does not make sense since version.parse() returns only Version with packaging 22

It uses the actions/cache@v3 to restore dependencies. I am not sure how to change those, so you may need to just add packaging==22 to the yml

@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat XS Extra small feature labels Dec 12, 2022
@piiq
Copy link
Contributor

piiq commented Dec 12, 2022

Does the Tests / General Linting create a fresh environment each time? The error does not make sense since version.parse() returns only Version with packaging 22

It creates a fresh env when the dep tree is re-solved

@jmaslek
Copy link
Collaborator

jmaslek commented Dec 12, 2022

Does the Tests / General Linting create a fresh environment each time? The error does not make sense since version.parse() returns only Version with packaging 22

It creates a fresh env when the dep tree is re-solved

The tests create a new env, but the linting uses the cache from gh actions. That has not been updated in 9 months.

I used this doc to open a new PR that will edit the cache when the poetry lock changes: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows

@jmaslek jmaslek merged commit 0e79f3f into OpenBB-finance:main Dec 12, 2022
@hjoaquim
Copy link
Contributor Author

@jmaslek @piiq thanks for the clarification!

@hjoaquim hjoaquim deleted the fix_packaging branch December 13, 2022 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug feat XS Extra small feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants