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

[FEAT] Added function to create nice labels and units #161

Merged
merged 6 commits into from
Dec 5, 2024

Conversation

MOchiara
Copy link
Member

@MOchiara MOchiara commented Dec 5, 2024

I have not added all possible variables to the dictionary (and I am not sure I am doing this the right way either).
I have not modified the labels for eleanors functions either

@MOchiara MOchiara linked an issue Dec 5, 2024 that may be closed by this pull request
@MOchiara MOchiara requested a review from callumrollo December 5, 2024 10:29
@MOchiara
Copy link
Member Author

MOchiara commented Dec 5, 2024

I have tried the possible new doc string format with

Notes
------
Original author: Chiara Monforte

We can see if we like it and if so I will change all the other functions

if var in label_dict:
units = f'{label_dict[var]["units"]}'
else:
units= f'{ds[var].units}'
Copy link
Member

Choose a reason for hiding this comment

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

recommend a further catch for variables without units

    if var in label_dict:
        return f'{label_dict[var]["units"]}'
    elif 'units' in ds[var].attrs:
        return f'{ds[var].attrs['units']}'
    else:
        return ""

glidertest/utilities.py Outdated Show resolved Hide resolved
var : The name of the variable to plot. Default is 'DOXY'.

Copy link
Member

Choose a reason for hiding this comment

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

As a general guidance, try not to add these stylistic fixes to unrelated code in a PR, it makes the review process more confusing! You can do a seperate PR that just fixes typos and style etc. Or, if you're feeling extra cool, you can check out python linters like astral

We can leave them in for this PR though :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok I did remove it haha

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@callumrollo
Copy link
Member

sorry I broke ur tests 😢

@MOchiara MOchiara merged commit 2635d57 into OceanGlidersCommunity:main Dec 5, 2024
10 checks passed
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.

Plot labels - Initial function with basic variables
2 participants