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

Make formatting consistent and remove references to the LiPD class #49

Merged
merged 2 commits into from
May 31, 2024

Conversation

khider
Copy link
Member

@khider khider commented Mar 5, 2024

This is mostly a cosmetic PR (e.g., assumes that all code is working) but will allow to start with a good template for all notebooks to actually fix code. Note that I don't expect the test to pass on this (shouldn't because of the run time of the correlation notebook).

What was actually done:

  • All templates were reformatted to go with "by" followed by author names with a link to ORCID. This fixes Pretty faces for notebook authors #35. The templates also include the preamble/data/Demosntration headers
  • Added watermark to all notebooks. Might be useful for users trying to reproduce our workflows.
  • Remove LiPD focused notebook (while moving all appropriate functionalities that were associated with the LiPD class to other notebooks) and make sure that we are no longer using the LiPD class anywhere. Fixes pyLipd integration into tutorials #23

@khider khider requested a review from CommonClimate March 5, 2024 01:23
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@CommonClimate CommonClimate left a comment

Choose a reason for hiding this comment

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

The LiPD class is still used in the correlation NB, without importing pylipd, so this cell fails for me:

D = LiPD()

D.load('../data/Ocn-Palmyra.Nurhati.2011.lpd')

timeseries,df = D.get_timeseries(D.get_all_dataset_names(),to_dataframe=True)

df

Is from pylipd.lipd import LiPD the only import that needs to be changed? I don't have xarray installed and I'm afraid to try as I fear it will break NumPy, Pandas and maybe others...

@khider
Copy link
Member Author

khider commented Mar 20, 2024

Works on my machine with pandas 2.1.4. I tried to pin the version but it gets upgraded.

@CommonClimate
Copy link
Contributor

Merging and hoping for the best

@CommonClimate CommonClimate merged commit 5ab9353 into main May 31, 2024
1 check failed
@khider khider deleted the Lipd branch June 18, 2024 17:41
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.

Pretty faces for notebook authors pyLipd integration into tutorials
2 participants