Skip to content

do not require nose for testing #5826

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
Merged
kevin-bates merged 6 commits into from
Oct 29, 2020
Merged

do not require nose for testing #5826

kevin-bates merged 6 commits into from
Oct 29, 2020

Conversation

@pgajdos
Copy link
Contributor

@pgajdos pgajdos commented Oct 22, 2020

nose is deprecated and will not work in future python versions.

@bnavigator bnavigator mentioned this pull request Oct 22, 2020
Open
kevin-bates
kevin-bates reviewed Oct 27, 2020
View reviewed changes
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

These changes look good - thank you! Only had the one comment regarding the print statements.

notebook/tests/test_paths.py Outdated
):
assert_regex(path, path_pat)
print(type(path))
print(type(path_pat))
Copy link
Member

@kevin-bates kevin-bates Oct 27, 2020

Choose a reason for hiding this comment

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

We should probably remove these print statements.

kevin-bates
kevin-bates approved these changes Oct 29, 2020
View reviewed changes
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Thanks @pgajdos.

@kevin-bates kevin-bates merged commit 7221718 into jupyter:master Oct 29, 2020
@hroncok
Copy link
Contributor

hroncok commented Jan 7, 2021

Note that nose appears still required via https://github.com/ipython/ipython_genutils/blob/fbcb174d06d6df1f8633014dd80fad8c45533c2e/ipython_genutils/testing/decorators.py#L162

@kevin-bates
Copy link
Member

kevin-bates commented Jan 8, 2021

Good catch @hroncok - thanks. Looks like the updates to notebook/tests/test_notebookapp_integration.py were missed. Any idea how nose is getting into the CI? I'm not seeing anything immediately, but it's definitely present in the pip freeze output.

At any rate, PR forthcoming to address the remaining references that pull in nose.

@hroncok
Copy link
Contributor

hroncok commented Jan 8, 2021

AFAIK nose comes pre-installed in Travis Python environments (I guess historically this was a good idea, not sure).

@kevin-bates kevin-bates mentioned this pull request Jan 8, 2021
Merged
@mcepl mcepl mentioned this pull request Jan 12, 2021
Open
@frenzymadness frenzymadness mentioned this pull request Feb 18, 2021
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Zsailer Zsailer Zsailer approved these changes

+1 more reviewer

@kevin-bates kevin-bates kevin-bates approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status:resolved-locked

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pgajdos @hroncok @kevin-bates @Zsailer