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

cleaner environment #30

Merged
merged 5 commits into from
Sep 13, 2023
Merged

cleaner environment #30

merged 5 commits into from
Sep 13, 2023

Conversation

RondeauG
Copy link
Collaborator

@RondeauG RondeauG commented Sep 13, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • HISTORY.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

  • The environment.yml file now only contains the bare minimum dependencies.
  • Removed the -docs and moved everything into a environment-dev.
  • Removed both requirements_(dev/docs).txt, since they were not used.

Does this PR introduce a breaking change?

  • Probably not?

Other information:

@@ -19,7 +19,7 @@ build:
- sphinx-apidoc -o docs/apidoc --private --module-first xhydro

conda:
environment: environment-docs.yml
environment: environment-dev.yml
Copy link
Collaborator

@Zeitsperre Zeitsperre Sep 13, 2023

Choose a reason for hiding this comment

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

Like I mentioned in person, there are a few ways of going about building the docs:

  1. We can install everything from the dev environment and evaluate notebooks (heavy on resources/memory, slower to build)
  2. We can mock the dependencies and install the library without any intention of running notebooks (faster, no gurantee that notebooks are running properly on RtD).
  3. We can do something fancier (like I developed in xclim).

Any of these approaches is fine. If we find that option 1 ends up being too slow, I can make something more custom resembling option 3 later...

"xclim>=0.43.0",
"zarr>=2.11.1",
]
requirements = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

@RondeauG and I spoke about this earlier. As we start adding content to xhydro, we should progressvely add requirements here. That way it doesn`t take so long to install/test in the early stages.

@@ -69,9 +40,7 @@
name="xhydro",
packages=find_packages(include=["xhydro", "xhydro.*"]),
test_suite="tests",
tests_require=test_requirements,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This field is deprecated and the pytest and setuptools maintainers suggest against using it. Good call.

More info: https://docs.pytest.org/en/latest/explanation/goodpractices.html#do-not-run-via-setuptools

@RondeauG RondeauG merged commit b6128e0 into main Sep 13, 2023
4 checks passed
@RondeauG RondeauG deleted the fix-23 branch September 13, 2023 20:55
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.

Cleaner environment
2 participants