Skip to content

MNT Add matplotlib and pandas as test dependencies#151

Merged
adrinjalali merged 1 commit intoskops-dev:mainfrom
BenjaminBossan:add-test-dependencies
Sep 26, 2022
Merged

MNT Add matplotlib and pandas as test dependencies#151
adrinjalali merged 1 commit intoskops-dev:mainfrom
BenjaminBossan:add-test-dependencies

Conversation

@BenjaminBossan
Copy link
Copy Markdown
Collaborator

Fixes #148

Description

matplotlib and pandas are currently not test dependencies (only docs) but actually, tests cannot run without them.

Now it should be possible to successfully run:

pip install skops[tests]
pytest skops

(I successfully tested locally with pip install -e .[tests])

Implementation

There was already the possibility to declare the same dependency for multiple extras by using ", " as separator. Personally, I would have preferred a tuple like so:

dependent_packages = {
    ...,
    ("pandas": ("1", ("docs", "tests"), None)),
}

but I guess the code is like this because sklearn does it the same.

Other

If merged, allows to simplify install in #147

Fixes skops-dev#148

matplotlib and pandas are currently not test dependencies (only docs)
but actually, tests cannot run without them.

Not it should be possible to successfully run:

pip install skops[tests]
pytest skops

(I tested locally with pip install -e .[tests])

Implementation

There was already the possibility to declare the same dependency for
multiple "extras" by using ", " as separator. Personally, I would have
preferred a tuple like so:

dependent_packages = {
    ...,
    ("pandas": ("1", ("docs", "tests"), None)),
}

but I guess the code is like this because sklearn does it the same.
@BenjaminBossan
Copy link
Copy Markdown
Collaborator Author

Forgot to ping: Ready for review @skops-dev/maintainers

Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

LGTM!

@adrinjalali adrinjalali changed the title Add matplotlib and pandas as test dependencies MNT Add matplotlib and pandas as test dependencies Sep 26, 2022
@adrinjalali adrinjalali merged commit 132925b into skops-dev:main Sep 26, 2022
@BenjaminBossan BenjaminBossan deleted the add-test-dependencies branch September 26, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Not all test dependencies covered

2 participants