Skip to content
This repository was archived by the owner on Sep 13, 2023. It is now read-only.

remote cmd fixes #103

Merged
merged 2 commits into from
Nov 8, 2021
Merged

remote cmd fixes #103

merged 2 commits into from
Nov 8, 2021

Conversation

mike0sv
Copy link
Contributor

@mike0sv mike0sv commented Nov 3, 2021

closes #31
closes #72

Signed-off-by: mike0sv mike0sv@gmail.com

closes #31
closes #72

Signed-off-by: mike0sv <mike0sv@gmail.com>
@mike0sv mike0sv requested a review from aguschin November 3, 2021 03:21
@mike0sv mike0sv self-assigned this Nov 3, 2021
@mike0sv mike0sv requested a review from a team November 3, 2021 03:21
Signed-off-by: mike0sv <mike0sv@gmail.com>
@codecov
Copy link

codecov bot commented Nov 4, 2021

Codecov Report

Merging #103 (6014ef1) into main (312c496) will increase coverage by 1.15%.
The diff coverage is 88.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   77.64%   78.79%   +1.15%     
==========================================
  Files          52       55       +3     
  Lines        3055     3282     +227     
==========================================
+ Hits         2372     2586     +214     
- Misses        683      696      +13     
Impacted Files Coverage Δ
mlem/pack/docker/context.py 34.61% <ø> (ø)
mlem/runtime/interface/base.py 81.52% <0.00%> (-1.82%) ⬇️
mlem/utils/module.py 77.30% <ø> (ø)
mlem/contrib/dvc.py 52.50% <52.50%> (ø)
mlem/core/base.py 68.88% <75.00%> (+18.88%) ⬆️
mlem/utils/github.py 80.48% <80.48%> (ø)
mlem/contrib/xgboost.py 97.29% <83.33%> (-1.37%) ⬇️
mlem/contrib/lightgbm.py 95.16% <85.71%> (-1.67%) ⬇️
mlem/contrib/numpy.py 90.90% <85.71%> (-1.02%) ⬇️
mlem/core/metadata.py 94.33% <87.50%> (+6.10%) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 383de28...6014ef1. Read the comment docs.

Copy link
Contributor

@aguschin aguschin left a comment

Choose a reason for hiding this comment

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

Cool! Seem to work now! 😎

Comment on lines 29 to 30
# TODO: do we really need this function?
# address in https://github.com/iterative/mlem/issues/4
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, this comment was suggesting that maybe we should unite resolve_fs() and get_fs() in one function. Not sure which is better, unite or leave them as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# TODO: do we really need this function?
# address in https://github.com/iterative/mlem/issues/4

In any case, we probably should remove this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be addressed in my next PR

@mike0sv mike0sv merged commit ed7a6dd into main Nov 8, 2021
@mike0sv mike0sv deleted the fix/remote-cmds branch November 8, 2021 13:22
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve git urls in mlem apply Resolve git urls in mlem pprint
2 participants