Skip to content

Conversation

@shcheklein
Copy link
Member

Migrate setup to pyproject.toml. Adds __version__ to the package. Needed to do dynamic checks (e.g. check that version is correct in the callback) and overall it's expected that packages have it.


Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@shcheklein shcheklein force-pushed the migrate-to-pyproject branch from 29c55cd to ac868e2 Compare May 29, 2023 02:16
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 57.14% and project coverage change: -0.10 ⚠️

Comparison is base (9bcf9b8) 96.20% compared to head (09497a3) 96.11%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
- Coverage   96.20%   96.11%   -0.10%     
==========================================
  Files          43       44       +1     
  Lines        2874     2881       +7     
  Branches      240      240              
==========================================
+ Hits         2765     2769       +4     
- Misses         65       68       +3     
  Partials       44       44              
Impacted Files Coverage Δ
src/dvclive/huggingface.py 93.10% <ø> (ø)
src/dvclive/keras.py 100.00% <ø> (ø)
src/dvclive/version.py 50.00% <50.00%> (ø)
src/dvclive/__init__.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@shcheklein shcheklein requested review from daavoo and skshetry May 29, 2023 02:29
Copy link
Contributor

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

Needed to do dynamic checks (e.g. check that version is correct in the callback) and overall it's expected that packages have it.

See #359 (comment) .

Do you have any reason not to use it in the callback?

I prefer that over adding the version, but I have no strong opinion.

@shcheklein
Copy link
Member Author

I prefer that over adding the version, but I have no strong opinion.

Also, don't have a strong opinion. I think it can be convenient since people seems to expect it still and most (?) of the package provide it. But I can remove it (I assume toml migration is still meaningful for us)

@daavoo daavoo merged commit 129afe5 into main May 30, 2023
@daavoo daavoo deleted the migrate-to-pyproject branch May 30, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants