Skip to content

Delete vscode configuration settings.#1228

Closed
copybara-service[bot] wants to merge 1 commit intomainfrom
test_760542016
Closed

Delete vscode configuration settings.#1228
copybara-service[bot] wants to merge 1 commit intomainfrom
test_760542016

Conversation

@copybara-service
Copy link

Delete vscode configuration settings.

PiperOrigin-RevId: 760542016
@Nush395
Copy link
Collaborator

Nush395 commented May 20, 2025

@theo-brown as a TORAX developer from the open source repo, are these configuration settings useful to you/something you see value in keeping?

@Nush395 Nush395 requested a review from theo-brown May 20, 2025 12:42
@theo-brown
Copy link
Collaborator

They're useful, but perhaps don't need to be part of the source code? e.g. I could move them to a section of the docs.

@Nush395
Copy link
Collaborator

Nush395 commented May 20, 2025

Good to know! (How) are you using them atm?

@theo-brown
Copy link
Collaborator

They get auto-detected by VSCode and then used as the "workspace settings", ie applied to anything edited or saved within the TORAX directory tree.

@theo-brown
Copy link
Collaborator

theo-brown commented May 22, 2025

One thing to note: currently my autoformat is still in slight disagreement, despite using this config. Things I've noticed:

  • isort overzealously sorting imports
  • Adding single blank line after imports and before code
  • Adding single blank line after docstrings
  • Slightly different linebreak/linewrap rules

I can try and fix this and then move this to a page in the developer docs?

@Nush395
Copy link
Collaborator

Nush395 commented Jun 5, 2025

One thing to note: currently my autoformat is still in slight disagreement, despite using this config. Things I've noticed:

  • isort overzealously sorting imports
  • Adding single blank line after imports and before code
  • Adding single blank line after docstrings
  • Slightly different linebreak/linewrap rules

I can try and fix this and then move this to a page in the developer docs?

If you are able to marry the mismatch that would be great! I'm a bit hesitant to move this to developer docs as I feel it would not be as discoverable there, it seems vscode is autopicking it up if it lives here which sounds ideal, wdyt?

@theo-brown
Copy link
Collaborator

Once #1329 is merged I'll revisit this - the differences in formatting were probably due to missing the changes that are being added to pyproject.toml

@Nush395
Copy link
Collaborator

Nush395 commented Jul 3, 2025

hi Theo, small update on this. 1329 is now in.

A workflow of using isort and pyink should now be sufficient for formatting. Mike also added a CI check for isort. Unfortunately we couldn't add a check for pyink yet because there is a bug in our internal formatter which leads to slight differences.

@Nush395
Copy link
Collaborator

Nush395 commented Jul 3, 2025

If you could check when you have a moment given that workflow if these changes are still relevant that would be super, thanks!

@theo-brown
Copy link
Collaborator

Closing in favour of #1346

@theo-brown theo-brown closed this Jul 3, 2025
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.

2 participants