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

Spellchecking with codespell #1576

Merged
merged 13 commits into from
Jan 9, 2024
Merged

Spellchecking with codespell #1576

merged 13 commits into from
Jan 9, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Jan 3, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adds codespell for checking word spellings
  • Configures codespell to ignore several French words and all current translations configurations.
  • Adds a pre-commit hook for performing these checks on commit

Does this PR introduce a breaking change?

Yes, codespell is now a development dependency.

Other information:

https://github.com/codespell-project/codespell

@Zeitsperre Zeitsperre added the standards / conventions Suggestions on ways forward label Jan 3, 2024
@Zeitsperre Zeitsperre added this to the v0.48.0 milestone Jan 3, 2024
@Zeitsperre Zeitsperre requested a review from a team January 3, 2024 16:10
@Zeitsperre Zeitsperre self-assigned this Jan 3, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added sdba Issues concerning the sdba submodule. CI Automation and Contiunous Integration docs Improvements to documenation indicators Climate indices and indicators labels Jan 3, 2024
Comment on lines +34 to 35
* The `codespell` library is now a development dependency for the `dev` installation recipe with configurations found within `pyproject.toml`. This is also now a linting step and integrated as a `pre-commit` hook. For more information, see the `codespell documentation <https://github.com/codespell-project/codespell>`_ (:pull:`1576`).

Copy link
Contributor

@coxipi coxipi Jan 9, 2024

Choose a reason for hiding this comment

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

Just to know: Is the dev installation automatically used when git cloning xclim and doing mamba install -n my_env -f environment.yml? I think xscen has environment_dev.yml, but not xclim, so how does this work here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's how we've organized xclim here. When someone installs xclim into a conda environment with the dev recipe, it pulls the development libraries from PyPI. This hasn't been an issue to date, but if we wanted to mirror the xscen approach, adding this file into the codebase is feasible.

Perhaps worth discussing in a separate issue?

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 see a problem with this organization, I just wanted to understand it, to be sure I'm not using a wrong version by error.

Copy link
Contributor

@coxipi coxipi left a comment

Choose a reason for hiding this comment

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

Nice to have. Seems to be working well, approved!

Note: There were some changes like x -> x. codespell does not make suggestions like this correct, just some unrelated completely manual changes while we're at it correct?

@Zeitsperre Zeitsperre added the approved Approved for additional tests label Jan 9, 2024
@Zeitsperre Zeitsperre merged commit e335ff4 into master Jan 9, 2024
15 of 16 checks passed
@Zeitsperre Zeitsperre deleted the spellchecking branch January 9, 2024 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests CI Automation and Contiunous Integration docs Improvements to documenation indicators Climate indices and indicators sdba Issues concerning the sdba submodule. standards / conventions Suggestions on ways forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants