Skip to content
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.

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

small documentation improvements #276

Merged
merged 50 commits into from
Dec 20, 2021
Merged

small documentation improvements #276

merged 50 commits into from
Dec 20, 2021

Conversation

yromanyshyn
Copy link
Contributor

No description provided.

@yromanyshyn yromanyshyn added the documentation modification of the documentation / readme's label Dec 20, 2021
@yromanyshyn yromanyshyn self-assigned this Dec 20, 2021
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

docs/source/conf.py Outdated Show resolved Hide resolved
sys.path.insert(0, str(PROJECT_DIR.absolute()))


import deepchecks.version
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't import this locally, can you remove it from here, or add it to the package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean that you get ImportError when trying to import deepchecks.version?

Copy link
Contributor

Choose a reason for hiding this comment

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

yup

makefile Outdated Show resolved Hide resolved
<<<<<<< HEAD
python = $(shell echo ${ext_py} | rev | cut -d '/' -f 1 | rev)
TESTDIR = $(shell realpath tests)
ENV = $(shell realpath venv)
Copy link
Contributor

Choose a reason for hiding this comment

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

this command doesn't work locally on my machine

Copy link
Contributor

Choose a reason for hiding this comment

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

it's a requirement to update requirements, I don't have it installed by default....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you use windows? (I pushed fix for this)

Copy link
Contributor

Choose a reason for hiding this comment

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

nope, I'm on mac

Copy link
Contributor Author

Choose a reason for hiding this comment

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

weird/ Did I understand you right that there is no realpath cmd in mac env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if so, I think this cmd will install it - brew install coreutils

@benisraeldan
Copy link
Contributor

right now we have duplicate notebooks, is there a way to reference them from outside the source directory? or should we move them to the source and keep them update and tested there?

@yromanyshyn
Copy link
Contributor Author

right now we have duplicate notebooks, is there a way to reference them from outside the source directory? or should we move them to the source and keep them update and tested there?

@benisraeldan

It looks like it is not possible to reference documents outside of the source dir in the rst/ at least I was not able to achieve this

Think we need to move notebooks to the docs/source folder

@yromanyshyn
Copy link
Contributor Author

right now we have duplicate notebooks, is there a way to reference them from outside the source directory? or should we move them to the source and keep them update and tested there?

@benisraeldan

It looks like it is not possible to reference documents outside of the source dir in the rst/ at least I was not able to achieve this

Think we need to move notebooks to the docs/source folder

@ItayGabbay any thoughts regarding that?

@ItayGabbay
Copy link
Member

I have a task on this. Just waiting for the PR in order to start.

@yromanyshyn yromanyshyn merged commit 4a2fa7c into main Dec 20, 2021
@delete-merged-branch delete-merged-branch bot deleted the doc-test-v2 branch December 20, 2021 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation modification of the documentation / readme's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants