Style Guide

[bourque]

This PR adds a project style guide

[greglucas]

This is really nice! Just some minor comments inline. I'd also suggest adding some comments about automatically checking things locally with pre-commit and how to get that set up. And also adding in some notes about black and ruff since those are what we are using to do our verification of these style "recommendations", meaning they are a little more than recommendations and leaning more towards requirements ;)

[bourque]

This is really nice! Just some minor comments inline. I'd also suggest adding some comments about automatically checking things locally with pre-commit and how to get that set up. And also adding in some notes about black and ruff since those are what we are using to do our verification of these style "recommendations", meaning they are a little more than recommendations and leaning more towards requirements ;)

@greglucas What is your workflow for setting up pre-commit and using black and ruff? I haven't used these before and i'm not familiar. I agree it would be good to add to this style guide but I just want to make sure I get it right.

[greglucas]

@greglucas What is your workflow for setting up pre-commit and using black and ruff? I haven't used these before and i'm not familiar. I agree it would be good to add to this style guide but I just want to make sure I get it right.

We should definitely add how to set those up and use them to the repository somewhere since they aren't in there yet, sorry about that! I made a rough cut of it in my docs PR:

https://github.com/IMAP-Science-Operations-Center/imap_processing/pull/15/files#diff-d7d2af9af9729ed1331913553f8232404d31175dfc64f8f0f3541b7f2b7bd417

with the gist being

pip install .[dev]
# Install the pre-commit hooks
pre-commit install

Then every git commit command will run those checks for you before allowing you to commit locally.

You can run black . to run black on all your files, or ruff . to run that on your files and also ruff --fix . to automatically fix the issues it finds and knows how to correct for you.

[bourque]

Awesome, thanks @greglucas!

[bourque]

@maxinelasp I added a (blank) section for poetry, you are welcome to add some notes there!

[bourque]

I am finished with this draft and this is ready for review! I would like to hear what you all think should be added/removed/changed.

[maxinelasp]

Would it be appropriate to add a note about testing or documentation here? Or is that not closely related to style? Actually, having a documentation style guide might be a good thing to add... (related to PR #15)

[bourque]

Yeah, now that #15 is merged and we have automated doc builds, I think it makes sense to add a note here about updating the necessary .rst files when applicable. I'll add that.

Beyond that, and beyond the mention of PEP257/numpydocs style, was there something else we could mention?

[maxinelasp]

I think just adding to update or write the documentation as required is all that's needed right now, although I will maybe just write up a documentation specific style guide

[maxinelasp]

Overall looks great, I added a few suggestions but feel free to take them or leave them

[greglucas]

I think the content here is great!

I just merged the docs PR, so we have a documentation section now. Should we move this into the docs/ subfolder? Or should we even include it in the sphinx build as another page/heading? I think we could keep it as Markdown and use Myst for the restructured text conversion if you want, or you could update this file to restructured text as well.

[tech3371]

This is looking great so far.

[bourque]

Overall looks great, I added a few suggestions but feel free to take them or leave them

Thanks @maxinelasp! This is a good opportunity to try out this robot ...

@all-contributors please add @maxinelasp for the code review!

🙏🏻

[allcontributors]

@bourque

I've put up a pull request to add @maxinelasp! 🎉

[bourque]

I think the content here is great!

I just merged the docs PR, so we have a documentation section now. Should we move this into the docs/ subfolder? Or should we even include it in the sphinx build as another page/heading? I think we could keep it as Markdown and use Myst for the restructured text conversion if you want, or you could update this file to restructured text as well.

@greglucas As long as the style guide is easily findable/accessible, I don't really have a strong opinion on where it should live, so I will refer to whatever you advise on this!

[greglucas]

@greglucas As long as the style guide is easily findable/accessible, I don't really have a strong opinion on where it should live, so I will refer to whatever you advise on this!

I think this depends on how we expect users to find this information in the long run. Will they be going through an HTML documentation site, or do we expect them to be browsing the repository for this information? I don't personally know the answer to that, I'd probably lean towards HTML myself, looking at some "contributing" section.

Just fine to add this now, and then move it around later once we actually have documentation builds showing up somewhere.

[bourque]

pre-commit.ci autofix