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

[DOCS] Add base document for contribution #115

Merged
merged 4 commits into from
Nov 15, 2024
Merged

Conversation

MOchiara
Copy link
Member

No description provided.

@MOchiara
Copy link
Member Author

This is a draft from https://github.com/bttger/contributing-gen

@callumrollo callumrollo marked this pull request as draft November 14, 2024 14:26
@callumrollo
Copy link
Member

Solid start! Marking this a draft to it won't get merged before it's ready. I think it's important that we all review this one

@MOchiara
Copy link
Member Author

Absolutely! Good point!

@eleanorfrajka
Copy link
Collaborator

eleanorfrajka commented Nov 14, 2024

I was also thinking of something along the lines of:
"
Getting started adding your own functionality.

  • Code is organised into .py files within glidertest/ and in notebooks/.ipynb. The *.py files include primarily functions (with their necessary packages imported) while the notebooks call these functions and display the plots generated. The *.py files are separated into broad categories of X, Y and Z. If you'd like to add a function to calculate some thing WIDGET and then to plot the result of the calculation, you will either want to write a function in tools.py and then a plotting function in plots.py. There are a couple exceptions: If it's a very simple calculation (mean, median, difference between two quantities), you might put the entire calculation within the plotting function. See for example plots.plot_abracadabra(). Or, if the calculation is more complicated, but is easily displayed by an existing function, then you might have a calculation function tools.calc_avrakadavra() and then use an existing plots.plot_histogram() to display it.

  • Once you've added a function, you can test it against one or two of the sample datasets in demo.ipynb. Does it have the same behaviour on those sample datasets as you expect?

  • Have you followed the conventions for naming your function? Generally, function names should be short, agnostic about the vehicle used, and understandable to Person X. We also loosely follow naming conventions to help the new user understand what a function might do (e.g., plotting functions in plots.py typically start with the name plot_blahblah() while calculations are calc_blahblah() and calculations with special outputs are compute_blahblah(). (continued).

  • Did you write a docstring? (see ref to http://blahblah.com). We also suggest including your name or GitHub handle under the original author heading.

  • There are also some basic error checking behaviours you should consider including. If you're plotting a particular variable, use the necessary_variables function to check whether or not the required variables are within the dataset passed to the function.

  • For plotting functions on a single axis, you should include as optional inputs the fig and ax, and return the same. For plotting functions with multiple or interrelated axes, perhaps fig and ax shouldn't be included as inputs, but can be provided as outputs for the user to make onward edits.

  • Unless otherwise required, suggest to pass an xarray dataset (as you get from loading an OG1 dataset) as the input. There are some parameters that can be additionally passed to carry out subsets on the data or select the variable of interest.

  • For plotting, see the guidance on uniformity (using standard colormaps, lines etc, and figure sizes and font sizes). These are all contained in the function blahblah.py, in case an individual user wants to change these to their preferences.
    "

@callumrollo callumrollo marked this pull request as ready for review November 15, 2024 10:58
@callumrollo
Copy link
Member

I have added and lightly editorialized Eleanor's suggestions, removed irrelevant sections and references and fixed all the links to point to parts of glidertest

CONTRIBUTING.md Outdated Show resolved Hide resolved
@MOchiara
Copy link
Member Author

Nice! Looks good to me!

@callumrollo callumrollo merged commit 71e33b0 into main Nov 15, 2024
10 checks passed
@eleanorfrajka
Copy link
Collaborator

Close issue #111 ?

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