Skip to content

Conversation

@StevePny
Copy link
Owner

@StevePny StevePny commented Oct 3, 2024

No description provided.

@StevePny StevePny self-assigned this Oct 3, 2024
@StevePny StevePny added the enhancement New feature or request label Oct 3, 2024
@StevePny StevePny requested a review from kysolvik October 3, 2024 00:09
@StevePny
Copy link
Owner Author

StevePny commented Oct 4, 2024

@kysolvik I've updated for the xarray refactor - it could use your review when you're ready.

@kysolvik
Copy link
Collaborator

kysolvik commented Oct 7, 2024

@StevePny Looks good to me! Two potential things before merging:

  1. Instead of help(data) at the top, maybe we should just do help(data.Lorenz63) since the full data module is quite large now and printing out all the docstrings/help info is quite long. I think I probably wrote help(data) a while ago and just never updated it.
  2. When plotting, we're indexing the ds['x'].data array object, instead of indexing the dataset itself. For example:
plt.plot(ds_l63['x'].data[:,0], ds_l63['x'].data[:,2], label="Original", color='gray')
plt.plot(ds_l63['x'].data[0,0], ds_l63['x'].data[0,2], marker='o', color='gray')

could be rewritten as:

plt.plot(ds_l63['x'].isel(index=0).data,  ds_l63['x'].isel(index=2).data label="Original", color='gray')
plt.plot(ds_l63['x'].isel(time=0, index=0), data ds_l63['x'].isel(time=0, index=2), marker='o', color='gray')

Either works, but I wonder if we should steer people towards using xarray's indexing instead of indexing the resulting numpy/jax array? What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants