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

README.md: Update installation instructions #5278

Merged
merged 6 commits into from
Feb 23, 2021

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Feb 18, 2021

Issue

Fixes #5112.

Instructions on the web site do not use virtual environments. I'd prefer keeping them that way. If anything, we can redirect advanced users to instructions on github.

Description of changes

Besides adding conda activate orange3 (and testing that all conda-based instructions work on my macOs), I rearranged the page

  • I moved installation to the top, and emphasized the easy installation via installer.
  • I replaced "steps" with a single code snippet to make it shorter, simpler, cleaner.
  • I separated installation of orange3 alone, and installation of orange-widget-base, orange-canvas-core and orange3, because only a few would be interested in the latter.

@codecov
Copy link

codecov bot commented Feb 18, 2021

Codecov Report

Merging #5278 (7ba7747) into master (50345c2) will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5278      +/-   ##
==========================================
+ Coverage   85.19%   85.36%   +0.17%     
==========================================
  Files         300      301       +1     
  Lines       60977    61833     +856     
==========================================
+ Hits        51950    52785     +835     
- Misses       9027     9048      +21     

@irgolic
Copy link
Member

irgolic commented Feb 18, 2021

Cool!

Can we add developer instructions for running, including tests? Also, commit these as PyCharm configurations?

Something like
Orange: python -m Orange.canvas -l 2 --no-splash --no-welcome --clear-widget-settings
Dark Orange: python -m Orange.canvas -l 2 --no-splash --no-welcome --clear-widget-settings --style=fusion:breeze-dark
Orange tests: unittest Orange.tests Orange.widgets.tests

README.md Outdated
To install Orange with [winget](https://docs.microsoft.com/en-us/windows/package-manager/winget/), run:
### Setting up for development of core widgets

Fork the repository by pressing the fork button in top-right corner of this page. Then execute the following lines (copy them one by one!):
Copy link
Member

Choose a reason for hiding this comment

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

Why do these need to be copied one by one? Can you append && to all lines up to the penultimate, and remove this?

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 you copy them all at once, the second line is used as the answer to Y/n prompt of the first.

Lines include YOUR-USERNAME, so they're not very copiable. Even more so in the next section.

Copy link
Member

@irgolic irgolic Feb 19, 2021

Choose a reason for hiding this comment

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

Might I suggest this?

# Add conda-forge to your channels for access to the latest release
conda config --add channels conda-forge

# Create and activate an environment for orange
yes | conda create python=3 --name orange3
conda activate orange3

# Install orange
conda install orange3

I can't think of a single time I followed instructions telling me to copy line by line, especially if presented with a copiable code block.

I don't think the yes would be too confusing, it's probably the simplest bash command out there. Perhaps just make sure it's clearly explained that the code block creates a new env.

@janezd janezd force-pushed the update-installation-instructions branch from d415cd4 to c7bd139 Compare February 18, 2021 16:29
@janezd
Copy link
Contributor Author

janezd commented Feb 18, 2021

Can we add developer instructions for running, including tests? Also, commit these as PyCharm configurations?

Something like
Orange: python -m Orange.canvas -l 2 --no-splash --no-welcome --clear-widget-settings
Dark Orange: python -m Orange.canvas -l 2 --no-splash --no-welcome --clear-widget-settings --style=fusion:breeze-dark
Orange tests: unittest Orange.tests Orange.widgets.tests

I added those (see "Running"). As for PyCharm configurations ... I'm not against, but let's discuss it.

@markotoplak markotoplak removed their assignment Feb 19, 2021
@markotoplak
Copy link
Member

@irgolic, merge at will.

- add Discord button to top
- some rewording/restyling
- add orange3-explain
- link style guide
@irgolic
Copy link
Member

irgolic commented Feb 19, 2021

I've made some adjustments, let me know how you feel about them.

If we can make all codeblocks easily copyable, I'll merge with pleasure :)

Sidenote, we should get https://github.com/irgolic/orange3/edit/README-shields/CONTRIBUTING.md up to speed. But keep in mind, I think the audience of that document is mainly people who write issues.

@janezd
Copy link
Contributor Author

janezd commented Feb 19, 2021

I agree that "copying line by line" is rather uncommon. :)

Add an instruction to replace replaceme with the name? (Besides -- if it appears only once, it doesn't really help. And the blocks that contain replaceme aren't really copyable.)

Anyway, I'll be happy with whatever you decide. I've no strong opinion here.

@irgolic
Copy link
Member

irgolic commented Feb 19, 2021

How's this?

@markotoplak merge at will :)

@markotoplak
Copy link
Member

@irgolic, I replaced yes | that probably would not work on Windows with an equivalent command-line option.

Copy and paste will not work in a Windows terminal, because variables there work differently, but I think the developers will manage. :) I would not like to have OS-specific instructions there.

I am merging this now. If you think the developers will be confused by the failing export command, we will need to use non-copyable command blocks.

@markotoplak markotoplak merged commit 3617a5b into biolab:master Feb 23, 2021
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.

Fix conda/pip installation instruction to include environment activation
3 participants