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.

Sign up for GitHub

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

Jump to bottom

Improve requirements parser #13912

Merged

carmocca merged 9 commits into master from ci/requirements-parser

Jan 12, 2023

Contributor

carmocca commented Jul 28, 2022 •

edited

Loading

What does this PR do?

Improves the requirement file parsing logic to properly support comments and pip arguments

Does your PR introduce any breaking changes? If yes, please list them.

None

Before submitting

Was this discussed/approved via a GitHub issue? (not for typos and docs)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together?
[n/a] Did you make sure to update the documentation with your changes? (if necessary)
Did you write any new necessary tests? (not for typos and docs)
Did you verify new and existing tests pass locally with your changes?
Did you list all the breaking changes introduced by this pull request?
[n/a] Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Is this pull request ready for review? (if not, please submit in draft mode)
Check that all items from Before submitting are resolved
Make sure the title is self-explanatory and the description concisely explains the PR
Add labels and milestones (and optionally projects) to the PR so it can be classified

cc @carmocca @akihironitta @Borda

carmocca added the ci label

carmocca added this to the all:next milestone

carmocca requested a review from williamFalcon as a code owner

July 28, 2022 15:37

carmocca self-assigned this

carmocca requested review from Borda, tchaton, awaelchli, justusschock, kaushikb11 and rohitgr7 as code owners

July 28, 2022 15:37

awaelchli approved these changes

View reviewed changes

akihironitta approved these changes

View reviewed changes

mergify bot added ready has conflicts and removed ready has conflicts labels

Borda reviewed

View reviewed changes

.actions/setup_tools.py Outdated Show resolved Hide resolved

.actions/setup_tools.py Outdated Show resolved Hide resolved

Borda requested changes

View reviewed changes

Member

Borda left a comment

I do not see http section, was it removed?

mergify bot removed the ready label

Contributor Author

carmocca commented Aug 3, 2022

I do not see http section, was it removed?

I believe pkg_resources.Requirement takes care of it. Do we install from an http link in any requirements file?

carmocca requested a review from Borda

August 3, 2022 16:11

Member

Borda commented Aug 3, 2022

I do not see http section, was it removed?

I believe pkg_resources.Requirement takes care of it. Do we install from an http link in any requirements file?

I think I tested in the past and pkg_resources left it (worse to check it again) and yes we have it docs for linking our template

Contributor Author

carmocca commented Aug 4, 2022

@Borda it does work:

Requirement.parse('pt-lightning-sphinx-theme@ https://github.com/Lightning-AI/lightning_sphinx_theme/archive/master.zip')

Borda approved these changes

View reviewed changes

Member

Borda left a comment

ok, but this one fails:

pkg_resources.Requirement.parse('https://github.com/Lightning-AI/lightning_sphinx_theme/archive/master.zip')

mergify bot added the ready label

Contributor Author

carmocca commented Aug 11, 2022

@Borda I'm getting errors during the package install step and I cannot find out why: https://github.com/Lightning-AI/lightning/runs/7773618982?check_suite_focus=true#step:9:56. I guess I "broke" an intermediate step and the lightning_app package is not getting installed for some reason, but I cannot seem to reproduce it locally.

Converting this to draft in the meantime to avoid an accidental merge.

carmocca marked this pull request as draft

August 11, 2022 00:01

Member

Borda commented Aug 11, 2022 •

edited

Loading

@Borda I'm getting errors during the package install step and I cannot find out why: https://github.com/Lightning-AI/lightning/runs/7773618982?check_suite_focus=true#step:9:56. I guess I "broke" an intermediate step and the lightning_app package is not getting installed for some reason, but I cannot seem to reproduce it locally.

Converting this to draft in the meantime to avoid an accidental merge.

interesting, it seems like the lightning_app is not installed :/
but it was downloaded...

carmocca mentioned this pull request

Setup: add requirement freeze for next major version #14480

Merged

12 tasks

carmocca removed the ready label

carmocca removed this from the all:next milestone

carmocca mentioned this pull request

Requirement files do not correspond to extra dependencies #15210

Open


          Update code

75277bf

carmocca force-pushed the ci/requirements-parser branch from 73528e1 to 75277bf Compare

January 5, 2023 01:38

carmocca added 3 commits

January 5, 2023 02:43


          LooseVersion


          Docstring

9daefa6


          String and text

102bd14

carmocca marked this pull request as ready for review

January 5, 2023 02:24

mergify bot added the ready label

Contributor

github-actions bot commented Jan 5, 2023 •

edited

Loading

⚡ Required checks status: All passing 🟢

Groups summary

🟢 pytorch_lightning: Tests workflow

Check ID	Status
pl-cpu (macOS-11, pytorch, 3.8, 1.11)	success	✅
pl-cpu (macOS-11, pytorch, 3.9, 1.12)	success	✅
pl-cpu (macOS-11, pytorch, 3.10, 1.13)	success	✅
pl-cpu (macOS-11, pytorch, 3.8, 1.10, oldest)	success	✅
pl-cpu (ubuntu-20.04, pytorch, 3.8, 1.10)	success	✅
pl-cpu (ubuntu-20.04, pytorch, 3.9, 1.11)	success	✅
pl-cpu (ubuntu-20.04, pytorch, 3.10, 1.12)	success	✅
pl-cpu (ubuntu-20.04, pytorch, 3.10, 1.13)	success	✅
pl-cpu (ubuntu-20.04, pytorch, 3.7, 1.10, oldest)	success	✅
pl-cpu (windows-2022, pytorch, 3.9, 1.11)	success	✅
pl-cpu (windows-2022, pytorch, 3.10, 1.12)	success	✅
pl-cpu (windows-2022, pytorch, 3.10, 1.13)	success	✅
pl-cpu (windows-2022, pytorch, 3.7, 1.10, oldest)	success	✅
pl-cpu (slow, macOS-11, pytorch, 3.7, 1.11)	success	✅
pl-cpu (slow, ubuntu-20.04, pytorch, 3.7, 1.11)	success	✅
pl-cpu (slow, windows-2022, pytorch, 3.7, 1.11)	success	✅
pl-cpu (macOS-11, lightning, 3.8, 1.13)	success	✅
pl-cpu (ubuntu-20.04, lightning, 3.8, 1.13)	success	✅
pl-cpu (windows-2022, lightning, 3.8, 1.13)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 pytorch_lightning: Azure GPU

Check ID	Status
pytorch-lightning (GPUs)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 pytorch_lightning: Azure HPU

Check ID	Status
pytorch-lightning (HPUs)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 pytorch_lightning: Azure IPU

Check ID	Status
pytorch-lightning (IPUs)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 pytorch_lightning: Docs

Check ID	Status
make-doctest (pytorch)	success	✅
make-html (pytorch)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 pytorch_lightning: Docker

Check ID	Status
build-cuda (3.9, 1.10, 11.3.1)	success	✅
build-cuda (3.9, 1.11, 11.3.1)	success	✅
build-cuda (3.9, 1.12, 11.6.1)	success	✅
build-cuda (3.9, 1.13, 11.7.1)	success	✅
build-hpu (1.5.0, 1.11.0)	success	✅
build-ipu (3.9, 1.10)	success	✅
build-NGC	success	✅
build-pl (3.9, 1.10, 11.3.1)	success	✅
build-pl (3.9, 1.11, 11.3.1)	success	✅
build-pl (3.9, 1.12, 11.6.1)	success	✅
build-pl (3.9, 1.13, 11.7.1)	success	✅
build-xla (3.7, 1.12)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 lightning_fabric: CPU workflow

Check ID	Status
fabric-cpu (macOS-11, fabric, 3.8, 1.11)	success	✅
fabric-cpu (macOS-11, fabric, 3.9, 1.12)	success	✅
fabric-cpu (macOS-11, fabric, 3.10, 1.13)	success	✅
fabric-cpu (macOS-11, fabric, 3.7, 1.10, oldest)	success	✅
fabric-cpu (ubuntu-20.04, fabric, 3.8, 1.10)	success	✅
fabric-cpu (ubuntu-20.04, fabric, 3.9, 1.11)	success	✅
fabric-cpu (ubuntu-20.04, fabric, 3.10, 1.12)	success	✅
fabric-cpu (ubuntu-20.04, fabric, 3.10, 1.13)	success	✅
fabric-cpu (ubuntu-20.04, fabric, 3.7, 1.10, oldest)	success	✅
fabric-cpu (windows-2022, fabric, 3.9, 1.11)	success	✅
fabric-cpu (windows-2022, fabric, 3.10, 1.12)	success	✅
fabric-cpu (windows-2022, fabric, 3.10, 1.13)	success	✅
fabric-cpu (windows-2022, fabric, 3.7, 1.10, oldest)	success	✅
fabric-cpu (macOS-11, lightning, 3.8, 1.13)	success	✅
fabric-cpu (ubuntu-20.04, lightning, 3.8, 1.13)	success	✅
fabric-cpu (windows-2022, lightning, 3.8, 1.13)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 lightning_fabric: Azure GPU

Check ID	Status
lightning-fabric (GPUs)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 lightning_app: Tests workflow

Check ID	Status
app-pytest (macOS-11, app, 3.8, latest)	success	✅
app-pytest (macOS-11, app, 3.8, oldest)	success	✅
app-pytest (macOS-11, lightning, 3.9, latest)	success	✅
app-pytest (ubuntu-20.04, app, 3.8, latest)	success	✅
app-pytest (ubuntu-20.04, app, 3.8, oldest)	success	✅
app-pytest (ubuntu-20.04, lightning, 3.9, latest)	success	✅
app-pytest (windows-2022, app, 3.8, latest)	success	✅
app-pytest (windows-2022, app, 3.8, oldest)	success	✅
app-pytest (windows-2022, lightning, 3.8, latest)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 lightning_app: Examples

Check ID	Status
app-examples (macOS-11, app, 3.9, latest)	success	✅
app-examples (macOS-11, app, 3.9, oldest)	success	✅
app-examples (macOS-11, lightning, 3.9, latest)	success	✅
app-examples (ubuntu-20.04, app, 3.9, latest)	success	✅
app-examples (ubuntu-20.04, app, 3.9, oldest)	success	✅
app-examples (ubuntu-20.04, lightning, 3.9, latest)	success	✅
app-examples (windows-2022, app, 3.9, latest)	success	✅
app-examples (windows-2022, app, 3.9, oldest)	success	✅
app-examples (windows-2022, lightning, 3.9, latest)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 lightning_app: Azure

Check ID	Status
App.cloud-e2e	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 lightning_app: Docs

Check ID	Status
make-doctest (app)	success	✅
make-html (app)	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 mypy

Check ID	Status
mypy	success	✅

These checks are required after the changes to .actions/assistant.py.

🟢 install

Check ID	Status
install-pkg (ubuntu-22.04, app, 3.7)	success	✅
install-pkg (ubuntu-22.04, app, 3.10)	success	✅
install-pkg (ubuntu-22.04, fabric, 3.7)	success	✅
install-pkg (ubuntu-22.04, fabric, 3.10)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.7)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.10)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.7)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.10)	success	✅
install-pkg (ubuntu-22.04, notset, 3.7)	success	✅
install-pkg (ubuntu-22.04, notset, 3.10)	success	✅
install-pkg (macOS-12, app, 3.7)	success	✅
install-pkg (macOS-12, app, 3.10)	success	✅
install-pkg (macOS-12, fabric, 3.7)	success	✅
install-pkg (macOS-12, fabric, 3.10)	success	✅
install-pkg (macOS-12, pytorch, 3.7)	success	✅
install-pkg (macOS-12, pytorch, 3.10)	success	✅
install-pkg (macOS-12, lightning, 3.7)	success	✅
install-pkg (macOS-12, lightning, 3.10)	success	✅
install-pkg (macOS-12, notset, 3.7)	success	✅
install-pkg (macOS-12, notset, 3.10)	success	✅
install-pkg (windows-2022, app, 3.7)	success	✅
install-pkg (windows-2022, app, 3.10)	success	✅
install-pkg (windows-2022, fabric, 3.7)	success	✅
install-pkg (windows-2022, fabric, 3.10)	success	✅
install-pkg (windows-2022, pytorch, 3.7)	success	✅
install-pkg (windows-2022, pytorch, 3.10)	success	✅
install-pkg (windows-2022, lightning, 3.7)	success	✅
install-pkg (windows-2022, lightning, 3.10)	success	✅
install-pkg (windows-2022, notset, 3.7)	success	✅
install-pkg (windows-2022, notset, 3.10)	success	✅

These checks are required after the changes to .actions/assistant.py.

Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

Borda reviewed

View reviewed changes

.actions/assistant.py Show resolved Hide resolved

.actions/assistant.py Outdated Show resolved Hide resolved

.actions/assistant.py Outdated Show resolved Hide resolved

carmocca added 2 commits

January 5, 2023 12:19


          Jirka's suggestions

828f0f6


          Merge branch 'master' into ci/requirements-parser

fe2bc6b

carmocca enabled auto-merge (squash)

January 9, 2023 15:53

Borda and others added 3 commits

January 10, 2023 21:20


          Merge branch 'master' into ci/requirements-parser

679327f


          Merge branch 'master' into ci/requirements-parser

f2bf47e


          Merge branch 'master' into ci/requirements-parser

d3d6163

carmocca merged commit b40b2ed into master

carmocca deleted the ci/requirements-parser branch

January 12, 2023 16:25

Borda mentioned this pull request

adding requirements parser Lightning-AI/utilities#107

Merged

4 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Borda Borda approved these changes

awaelchli awaelchli approved these changes

akihironitta akihironitta approved these changes

williamFalcon Awaiting requested review from williamFalcon

tchaton Awaiting requested review from tchaton

justusschock Awaiting requested review from justusschock justusschock is a code owner

kaushikb11 Awaiting requested review from kaushikb11

rohitgr7 Awaiting requested review from rohitgr7

Labels

ci ready