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

Update API documentation and switch to @properties for some Descriptor attributes. #22

Merged
merged 5 commits into from
Dec 19, 2024

Conversation

andrewdnolan
Copy link
Collaborator

@andrewdnolan andrewdnolan commented Dec 19, 2024

This PR:

  1. Improves the documentation, switch to numpy style docstrings. The primary improvements are made for the Descriptor class, which has improved formatting and more detailed information (for both the class itself and it's attributes).

  2. I've switched the projection and latlon attributes to be properties. I did this in order to move some logic outside of the method construct to abstract things a little. Most importantly the coordinate arrays (from the minimal dataset) transformation has been moved to the Descriptor.projection setter method so that reprojection to a new map projection after Descriptor initialization is now possible, although not really advised. These abstraction should also help set the groundwork for planar-periodic mesh support.

@andrewdnolan andrewdnolan added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 19, 2024
@andrewdnolan andrewdnolan requested a review from xylar December 19, 2024 02:32
@andrewdnolan andrewdnolan self-assigned this Dec 19, 2024
Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

@andrewdnolan, these changes look great to me! I have some picky suggestions about single-line docstrings below.

I built the docs locally and looked through them. Everything looked as expected -- very clean!

mosaic/descriptor.py Outdated Show resolved Hide resolved
mosaic/descriptor.py Outdated Show resolved Hide resolved
mosaic/descriptor.py Outdated Show resolved Hide resolved
mosaic/descriptor.py Outdated Show resolved Hide resolved
andrewdnolan and others added 3 commits December 19, 2024 11:54
for certain Descriptor attributes, that need additional logical in the
setter methods.
Correct the formatting for single line doc strings.

Co-authored-by: Xylar Asay-Davis <[email protected]>
@andrewdnolan
Copy link
Collaborator Author

Testing

Here's a demonstration of the reprojection capabilities enable by the Descriptor.projection setter method:

import cartopy.crs as ccrs
import mosaic
import matplotlib.pyplot as plt
import xarray as xr

def make_figure(ds, descriptor):

    fig, ax = plt.subplots(
        figsize=(9,7), constrained_layout=True, subplot_kw=dict(projection=descriptor.projection)
    )

    collection = mosaic.polypcolor(ax, descriptor, ds.indexToCellID, antialiaseds=True)

    ax.coastlines()
    fig.colorbar(collection, fraction=0.1, shrink=0.5, label="Cell Index");

    return fig, ax

ds = mosaic.datasets.open_dataset("QU.240km")

projection_one = ccrs.InterruptedGoodeHomolosine()
projection_two = ccrs.LambertConformal()
transform = ccrs.PlateCarree()

descriptor = mosaic.Descriptor(ds, projection_one, transform)

# first figure with original projection 
fig_one, ax_one = make_figure(ds, descriptor)

# switch the projection of the descriptor 
descriptor.projection = projection_two

# second figure with new projection
fig_two, ax_two = make_figure(ds, descriptor)

plt.show()

Figure One:

Screenshot 2024-12-19 at 11 59 54 AM

Figure Two:

Screenshot 2024-12-19 at 12 00 27 PM

@andrewdnolan andrewdnolan merged commit 7e0d993 into E3SM-Project:main Dec 19, 2024
5 checks passed
@andrewdnolan andrewdnolan deleted the doc_updates branch December 19, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants