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.

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

[build] move requirements to pyproject.toml #1031

Merged
merged 40 commits into from
Aug 31, 2022
Merged

[build] move requirements to pyproject.toml #1031

merged 40 commits into from
Aug 31, 2022

Conversation

felixdittrich92
Copy link
Contributor

@felixdittrich92 felixdittrich92 commented Aug 25, 2022 •
edited

This PR:

  • move requirements to pyproject.toml
  • update CI
  • update specifier

related to: #957

api updates are borrowed from https://github.com/frgfm/Holocron/tree/main/api thanks @frgfm 😅 have added comment in makefile 👍

@felixdittrich92 felixdittrich92 added type: enhancement Improvement topic: build Related to dependencies and build topic: ci Related to CI labels Aug 25, 2022
@felixdittrich92 felixdittrich92 added this to the 0.6.0 milestone Aug 25, 2022
@codecov
Copy link

codecov bot commented Aug 25, 2022 •
edited

Codecov Report

Merging #1031 (fef6c57) into main (61d0f1c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1031   +/-   ##
=======================================
  Coverage   94.93%   94.93%           
=======================================
  Files         135      135           
  Lines        5590     5590           
=======================================
  Hits         5307     5307           
  Misses        283      283
Flag Coverage Δ
unittests 94.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/transforms/modules/base.py 94.59% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@felixdittrich92 felixdittrich92 changed the title [DRAFT] [build] move requirements to pyproject.toml [build] move requirements to pyproject.toml Aug 25, 2022
@felixdittrich92
Copy link
Contributor Author

felixdittrich92 commented Aug 25, 2022 •
edited

Update: will check this today again :)

@felixdittrich92 felixdittrich92 marked this pull request as ready for review August 25, 2022 21:31
@felixdittrich92 felixdittrich92 marked this pull request as draft August 26, 2022 04:10
@felixdittrich92 felixdittrich92 marked this pull request as ready for review August 26, 2022 08:38
@felixdittrich92
Copy link
Contributor Author

ok now we got it 😅

@felixdittrich92 felixdittrich92 self-assigned this Aug 26, 2022
odulcy-mindee
odulcy-mindee reviewed Aug 26, 2022
View reviewed changes
.github/workflows/docker.yml Outdated Show resolved Hide resolved
.github/workflows/docker.yml Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/style.yml Show resolved Hide resolved
api/Dockerfile Show resolved Hide resolved
api/docker-compose.yml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
.github/workflows/docker.yml Outdated Show resolved Hide resolved
@felixdittrich92 felixdittrich92 mentioned this pull request Aug 30, 2022
Merged
odulcy-mindee
odulcy-mindee approved these changes Aug 31, 2022
View reviewed changes
Copy link
Collaborator

@odulcy-mindee odulcy-mindee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! We'll try to factorize the code between CI and Makefile another time, that's not that urgent

pyproject.toml Show resolved Hide resolved
@felixdittrich92 felixdittrich92 merged commit bf356a3 into mindee:main Aug 31, 2022
@felixdittrich92 felixdittrich92 deleted the move-requirements branch August 31, 2022 13:18
frgfm
frgfm reviewed Sep 5, 2022
View reviewed changes
Copy link
Collaborator

@frgfm frgfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late review, but I think there is a slight problem with the "dev" extra

pyproject.toml
Comment on lines +90 to +94
"python-doctr[tf]",
"python-doctr[torch]",
"python-doctr[testing]",
"python-doctr[quality]",
"python-doctr[docs]"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's dangerous because it will install the last published release. But in dev mode, you'll want to have the version of which the code is on your laptop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frgfm that's a really good catch ... do you know an alternative way without rewriting all deps ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope that's why I rewrite all deps 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this is not a dynamic build system yet, so that's not possible by design to the best of my knowledge)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you like to open a PR for this ? 😅 Unfortunately im blocked until Wednesday (and the free time i have is currently filled with implementing ViT and ViTSTR in doctr 😅 )

This was referenced Sep 5, 2022
Merged
Closed
@felixdittrich92 felixdittrich92 mentioned this pull request Sep 26, 2022
Closed
85 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frgfm frgfm frgfm left review comments

@odulcy-mindee odulcy-mindee odulcy-mindee approved these changes

@aminemindee aminemindee Awaiting requested review from aminemindee

@charlesmindee charlesmindee Awaiting requested review from charlesmindee

Assignees

@felixdittrich92 felixdittrich92

Labels
topic: build Related to dependencies and build topic: ci Related to CI type: enhancement Improvement
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@felixdittrich92 @frgfm @odulcy-mindee