Skip to content

Docs: Remove env_developer.yml from install guide #727

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
peanutfun merged 2 commits into from
May 23, 2023
Merged

Docs: Remove env_developer.yml from install guide #727

peanutfun merged 2 commits into from
May 23, 2023

Conversation

@tovogt
Copy link
Collaborator

@tovogt tovogt commented May 23, 2023 •
edited
Loading

Changes proposed in this PR:

PR Author Checklist

PR Reviewer Checklist

peanutfun
peanutfun requested changes May 23, 2023
View reviewed changes
Copy link
Member

@peanutfun peanutfun left a comment

Choose a reason for hiding this comment

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

@tovogt Thanks a lot for this fix, I missed the part where the old env_developer.yml was still used. However, the information on dev and extras in general was moved into its own section, see https://climada-python.readthedocs.io/en/latest/guide/install.html#install-developer-dependencies-optional. Did you miss that part, by chance? I don't think we should display this information twice, and I also think that installing the dev requirements should not be the default instruction.

@tovogt
Copy link
Collaborator Author

tovogt commented May 23, 2023

Ah, okay, I missed that part. I thought I was in the section about developer instructions already. And the dev requirements used to be the default here before. But I'm fine with removing the extra part.

@tovogt tovogt changed the title DOC: Mention [dev] extra in install guide Docs: Remove env_developer.yml from install guide May 23, 2023
@tovogt
Copy link
Collaborator Author

tovogt commented May 23, 2023 •
edited
Loading

I added your suggested changes.

Are you aware that the Install Developer Dependencies (Optional) section claims that [test] is required to run the tests, but you already run tests as an example at the end of the previous Advanced Instructions where this extra is not installed yet?

@peanutfun
Copy link
Member

peanutfun commented May 23, 2023

@tovogt I admit, this is a bit imprecise. Executing python -m unittest climada.engine.test.test_impact does not require the test extras. These extras mostly contain "test tools" like coverage etc.

@peanutfun
Copy link
Member

peanutfun commented May 23, 2023

I tried to mention this somewhat explicitly:

Building the documentation and running the entire test suite of CLIMADA requires additional dependencies

(emphasis by me)

@tovogt Do you think this is clear enough or should we change something about that?

@tovogt
Copy link
Collaborator Author

tovogt commented May 23, 2023

Don't worry, I think it's clear enough. Thanks for considering my concerns :)

@peanutfun
Copy link
Member

peanutfun commented May 23, 2023

Great, thanks a lot for taking your time and fixing the docs! 🎉

@peanutfun peanutfun merged commit 54dd0fc into develop May 23, 2023
@emanuel-schmid emanuel-schmid deleted the feature/doc_install_dev_extra branch May 23, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peanutfun peanutfun peanutfun approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tovogt @peanutfun