Skip to content

Expand contributing guide with specific style guidelines #345

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

K-Meech
Copy link
Contributor

@K-Meech K-Meech commented Jun 10, 2025

Closes #296

This PR expands the contributing guide, adding sections for general / this lesson as suggested by @tobyhodges on the contributing guide issue. I copied over relevant content from the collaborative lesson dev contributing guide which was linked on the issue. I also added specific style suggestions based on various closed issues/PRs from this repository.

Copy link

github-actions bot commented Jun 10, 2025

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/datacarpentry/image-processing/compare/md-outputs..md-outputs-PR-345

The following changes were observed in the rendered markdown documents:


What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2025-06-15 14:23:47 +0000

github-actions bot pushed a commit that referenced this pull request Jun 10, 2025
@K-Meech K-Meech requested a review from a team June 10, 2025 08:59
Copy link
Member

@tobyhodges tobyhodges left a comment

Choose a reason for hiding this comment

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

This looks great to me, thanks so much @K-Meech. Excited to see what the other lesson Maintainers think.

Copy link
Contributor

@marcodallavecchia marcodallavecchia left a comment

Choose a reason for hiding this comment

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

This looks great @K-Meech! Thanks a lot! I just left a small comment regarding the Python vs any code for the source code of a potential contribution.

CONTRIBUTING.md Outdated
or spent making a change that will not be accepted by the Maintainers.

#### Style / content guidelines
- If you add an image / figure that was generated from python code, please include this
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep it general for any programming language? Is there a specific reason for which we would reject a contribution of an image created with Matlab code or ImageJ macro?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should reject it, but perhaps we don't want to encourage it? 😜

For example, is it straightforward to document the equivalent of

### USAGE
# The script was written in Python 3.12 and required the following Python packages:
# - numpy==2.2.3
# - scipy==1.15.2
# - matplotlib==3.10.1
# - tqdm==4.67.1
#
# The script can be executed with
# $ python create_blur_animation.py
# The output animation will be saved directly in the fig folder where the markdown lesson file will pick it up
###

for an ImageJ macro?

CONTRIBUTING.md Outdated
or spent making a change that will not be accepted by the Maintainers.

#### Style / content guidelines
- If you add an image / figure that was generated from python code, please include this
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should reject it, but perhaps we don't want to encourage it? 😜

For example, is it straightforward to document the equivalent of

### USAGE
# The script was written in Python 3.12 and required the following Python packages:
# - numpy==2.2.3
# - scipy==1.15.2
# - matplotlib==3.10.1
# - tqdm==4.67.1
#
# The script can be executed with
# $ python create_blur_animation.py
# The output animation will be saved directly in the fig folder where the markdown lesson file will pick it up
###

for an ImageJ macro?

@uschille
Copy link
Contributor

Thanks @K-Meech, this is great! I second the minor amendments proposed.

github-actions bot pushed a commit that referenced this pull request Jun 15, 2025
@K-Meech
Copy link
Contributor Author

K-Meech commented Jun 15, 2025

Thanks all for the input! I've now updated the text with all the suggested changes. Could someone take a final look through - and check all looks good?

@marcodallavecchia - good point about the image/figure source code. For now, I've left the comment as Python only - mainly in the interest of ease of maintenance. As this lesson focuses on Python, it's likely that not all maintainers (or future maintainers!) will be familiar with ImageJ macro (for example), which could make modifying / updating this source code difficult. Very happy to re-visit this though, if a non-python figure is added in the future + if anyone else has a different opinion on this 😄

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.

Contributing guide should be customised further
5 participants