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

Add intersphinx_registry and toml-sort #1314

Merged
merged 9 commits into from
Oct 4, 2024
Merged

Conversation

mscheltienne
Copy link
Member

Add toml-sort and intersphinx_registry to the tooling chain.
Closes #1300

Copy link

codecov bot commented Oct 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.46%. Comparing base (65da1ae) to head (751ab6e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1314   +/-   ##
=======================================
  Coverage   97.46%   97.46%           
=======================================
  Files          40       40           
  Lines        8840     8840           
=======================================
  Hits         8616     8616           
  Misses        224      224           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

I suggest adding the following to pyproject.toml:

[tool.tomlsort]
sort_table_keys = true
sort_inline_tables = true
sort_inline_arrays = true
spaces_before_inline_comment = 2
spaces_indent_inline_array = 4
trailing_comma_inline_array = true

the sort_* settings will make order fully deterministic, and the other settings will make the result more similar to how ruff formats python code, which IMO makes manual edits slightly easier (e.g., hitting TAB in the IDE will yield the right number of spaces by default)

@mscheltienne
Copy link
Member Author

mscheltienne commented Oct 4, 2024

I was certain I putted:

[tool.tomlsort]
all = true
ignore_case = true
trailing_comma_inline_array = true

But now I can't find it..
This config is also deterministic and IIRC all replaces your 3 sort lines. For the spacing, I would argue it's a configuration of the IDE. If I hit TAB in VSCode in a pyproject.toml, I get a spacing of 2, not 4.

pyproject.toml Outdated Show resolved Hide resolved
@drammock
Copy link
Member

drammock commented Oct 4, 2024

This config is also deterministic and IIRC all replaces your 3 sort lines. For the spacing, I would argue it's a configuration of the IDE. If I hit TAB in VSCode in a pyproject.toml, I get a spacing of 2, not 4.

yep, confirmed that all is equiv to the 3 sort lines I suggested. (I was skimming docs quickly and assumed it meant "format all discovered files" or something).

Your point about spacing is true --- users can configure their IDEs to do tabs differently for different file formats. But if we use the setting I suggested, they won't need to do that... though I guess it's possible that my suggested setting will lead folks to need to undo their 2-space tab settings?

For other settings though (e.g., 2 spaces before inline comments) it's less about IDEs and more about muscle memory: chances are if I'm typing an inline comment I'll use 2 spaces, and when reading code I'm used to things being looking a certain way (I'm guessing that's why you added trailing comma --- after months/years of using black/ruff, it's starting to look "wrong" if it's not there?)

If you're not convinced that's OK. I don't insist on anything other than the sorting, which you've already done.

@mscheltienne
Copy link
Member Author

For other settings though (e.g., 2 spaces before inline comments) it's less about IDEs and more about muscle memory: chances are if I'm typing an inline comment I'll use 2 spaces, and when reading code I'm used to things being looking a certain way (I'm guessing that's why you added trailing comma --- after months/years of using black/ruff, it's starting to look "wrong" if it's not there?)

I completely agree and wil ladd the 2 spaces before inline comments at least ;)

Your point about spacing is true --- users can configure their IDEs to do tabs differently for different file formats. But if we use the setting I suggested, they won't need to do that... though I guess it's possible that my suggested setting will lead folks to need to undo their 2-space tab settings?

My issue here, is that TBH, I don't think I actually configured anything here and yet that's the default I got (it might be from the Even Better TOML externsion that I have installed). So in this case, we have 2 different 'default' behavior, which is not great. I'm enclined to keep the default from toml-sort for this one.

@drammock drammock merged commit 5bfb9b4 into mne-tools:main Oct 4, 2024
23 checks passed
@mscheltienne mscheltienne deleted the dev branch October 4, 2024 17:53
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.

Use intersphinx_registry
4 participants