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

Add some diagnostic plots + enhance phrasing in Demo notebook #183

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

navidcy
Copy link
Contributor

@navidcy navidcy commented Aug 23, 2024

I added some plots in the notebook. But I have forgotten how to actually run this notebook so that the plots appear and save them...

@angus-g or @ashjbarnes could you remind me how to do that?

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@navidcy
Copy link
Contributor Author

navidcy commented Aug 23, 2024

I actually remembered now a whole discussion about how to do it with the docker which I never managed to do...

@navidcy navidcy changed the title Add some diagnostic plots in Demo notebook Add some diagnostic plots + enhance phrasing in Demo notebook Aug 23, 2024
@navidcy
Copy link
Contributor Author

navidcy commented Aug 23, 2024

I see there is an issue with matplotlib. I can drop that bit and just use xarray's plot functionality. We shouldn't include matplotlib as a dependency of the regional-mom6 package but I'm not sure if we can include it in the docker env. But if that's cumbersome let's just not do that -- just let me know and I'll edit accordingly.

"### Modular workflow!\n",
"\n",
"After constructing your `expt` object, if you don't like the default `hgrid` and `vgrid` you can simply modify and then save them back into the `expt` object. However, you'll then also need to save them to disk again. For example:\n",
"After constructing our `expt` object, if we don't like default horizontal and vertical grids (`hgrid` and `vgrid`) we can modify and then save them back into the `expt` object. However, we will also need to save them to disk again. For example:\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"After constructing our `expt` object, if we don't like default horizontal and vertical grids (`hgrid` and `vgrid`) we can modify and then save them back into the `expt` object. However, we will also need to save them to disk again. For example:\n",
"After constructing our `expt` object, if we don't like the default horizontal and vertical grids (`hgrid` and `vgrid`) we can modify and then save them back into the `expt` object. However, we will also need to save them to disk again. For example:\n",

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's easiest to just include matplotlib in the package. No, it's not needed by the package, but it's the first thing people will install when they make a new env and realise it's missing

Copy link
Contributor Author

@navidcy navidcy Oct 5, 2024

Choose a reason for hiding this comment

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

I don't think we should have matplotlib as a dependency.

But I was googling this a bit and saw that there is a way to have "optional dependencies"? See the extras_require kwarg:

https://stackoverflow.com/questions/41268863/what-is-the-difference-between-extras-require-and-install-requires-in-se/45043494#45043494

That might be a nice way? @angus-g have you seen this before? You familiar with how it works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from chatGPT:

If you want to use optional dependencies with setuptools but without a setup.py file, you can still achieve this by using the pyproject.toml configuration. This is in line with the modern PEP 517 and PEP 518 standards, which define the use of pyproject.toml to manage build tools like setuptools.

Here’s how you can set it up using setuptools in pyproject.toml:

Step-by-Step Setup Using pyproject.toml with setuptools

  1. Create or Update pyproject.toml:

    You need to configure setuptools as your build backend in the pyproject.toml file.

    Here's an example:

    [build-system]
    requires = ["setuptools", "wheel"]
    build-backend = "setuptools.build_meta"
    
    [project]
    name = "yourpackage"
    version = "0.1.0"
    description = "A package with optional dependencies"
    authors = [
        { name = "Your Name", email = "[email protected]" }
    ]
    dependencies = [
        # Add essential dependencies here, e.g.
        "requests",
    ]
    
    [project.optional-dependencies]
    visualization = ["matplotlib"]
    • [build-system] specifies that you're using setuptools and wheel for your package's build system.
    • [project] defines the core metadata of your project (name, version, etc.).
    • dependencies lists the required dependencies for your package.
    • [project.optional-dependencies] is where you define optional features like visualization with its corresponding dependencies.
  2. Instruct Users:

    After setting up pyproject.toml, users can install the package with optional dependencies as follows:

    pip install yourpackage[visualization]

    This command will install the core package plus the optional matplotlib dependency under the visualization group.

  3. Handle Conditional Imports:

    In your code, you can use try-except to conditionally import the optional dependency:

    try:
        import matplotlib.pyplot as plt
    except ImportError:
        raise ImportError("Matplotlib is required for visualization. Please install it with `pip install yourpackage[visualization]`.")

Benefits of Using pyproject.toml with setuptools:

  • No setup.py needed: pyproject.toml serves as the modern alternative to setup.py and provides a clean way to configure optional dependencies.
  • Standardized build configuration: This method complies with PEP 517 and PEP 518, ensuring compatibility with modern Python tooling.
  • Optional dependencies in a clean format: You can define multiple optional groups (e.g., visualization, testing, docs), and users can choose which extras to install.

Example for Multiple Optional Dependencies

If you want to define multiple optional features (e.g., visualization, docs, dev), you can do it like this:

[project.optional-dependencies]
visualization = ["matplotlib"]
docs = ["sphinx", "myst-parser"]
dev = ["pytest", "black"]

Now, users can install specific extras by running:

pip install yourpackage[visualization]
# or
pip install yourpackage[dev,docs]

This setup with pyproject.toml using setuptools provides flexibility and avoids the need for setup.py while still giving you full control over your optional dependencies. Let me know if you need help with any specific part!

Copy link
Collaborator

@angus-g angus-g Oct 5, 2024

Choose a reason for hiding this comment

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

I don't think we should have matplotlib as a dependency.

Agreed.

That might be a nice way? @angus-g have you seen this before? You familiar with how it works?

I have used this a few times before, but I'm not sure how it interacts with conda... I don't think it's unreasonable that somebody would already have matplotlib installed if they wanted to plot stuff anyway? Plotting isn't really the job of this 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.

hm.... probably need to have different environment yml files and mix and match... oh well..

@navidcy
Copy link
Contributor Author

navidcy commented Oct 30, 2024

We can close this PR I think

@ashjbarnes
Copy link
Collaborator

I wouldn't want to waste the work you put into the diagnostics! Could we leave it open to remind us that it's here, and incorporate these ideas into the new example notebook that @helenmacdonald and I will work on? Then close it when we merge things together?

@navidcy
Copy link
Contributor Author

navidcy commented Oct 31, 2024

Deal

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.

3 participants