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

Feature/better signatures #62

Merged
merged 29 commits into from
Oct 8, 2021
Merged

Feature/better signatures #62

merged 29 commits into from
Oct 8, 2021

Conversation

mike0sv
Copy link
Contributor

@mike0sv mike0sv commented Oct 6, 2021

closes #21

aguschin and others added 26 commits September 30, 2021 17:23
Signed-off-by: mike0sv <mike0sv@gmail.com>
* Specify issues for todos

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

* create few issues and add links to them in code, making tests work with `example-mlem` repo

Co-authored-by: Alexander Guschin <1aguschin@gmail.com>
* add GHA to release on pypi, fix some tests
Signed-off-by: mike0sv <mike0sv@gmail.com>
# Conflicts:
#	mlem/api/commands.py
#	mlem/contrib/xgboost.py
#	mlem/core/dataset_type.py
#	mlem/core/requirements.py
#	mlem/utils/root.py
closes #21

Signed-off-by: mike0sv <mike0sv@gmail.com>
@mike0sv mike0sv requested a review from a team October 6, 2021 18:57
@mike0sv mike0sv self-assigned this Oct 6, 2021
Signed-off-by: mike0sv <mike0sv@gmail.com>
@aguschin aguschin self-requested a review October 8, 2021 08:45
@aguschin aguschin added breaking-change This is going to break users code core Related to things in mlem.core labels Oct 8, 2021
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! I've read the code and left some questions.

),
)

expected_requirements = {"catboost", "pandas", "numpy"}
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, why we have pandas here and don't have it in tests for sklearn, lighgbm, xgboost? Does it come from "install_requires" each library has?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's because we use pandas data for input

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, so ideally we would have to add tests for both numpy and pandas data, right?

@codecov
Copy link

codecov bot commented Oct 8, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@1442e3e). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #62   +/-   ##
=======================================
  Coverage        ?   75.97%           
=======================================
  Files           ?       52           
  Lines           ?     3038           
  Branches        ?        0           
=======================================
  Hits            ?     2308           
  Misses          ?      730           
  Partials        ?        0           

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 1442e3e...7ff0973. 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.

🔥

@mike0sv mike0sv merged commit 5fb3570 into main Oct 8, 2021
@mike0sv mike0sv deleted the feature/better-signatures branch October 8, 2021 13:04
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
breaking-change This is going to break users code core Related to things in mlem.core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve infering model methods and signatures
2 participants