Address Ruff Rule C408 in astropy/io Sub-package - #14841

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

• Do the proposed changes actually accomplish desired goals?
• Do the proposed changes follow the Astropy coding guidelines?
• Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
• Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
• Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
• Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
• Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
• Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
• Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
• At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

Thanks @Mubin17! This all looks very good. And it's wise to tackle these corrections on a sub package basis.
Side note, we recommend installing pre-commit on your computer (see https://pre-commit.com/#usage), which should automatically apply black and ensure that the pre-commit CI always passes.
Also, now that C408 is enforced, let's keep it enforced!
In .ruff.toml, if you could remove C408 from the ignore list (

@Mubin17, I found a configuration error in the .ruff.toml and am pushing a fix as part of #14843. Maybe hold off on editing .ruff.toml until that PR is in.

Modifications to io.misc are trivial, no issues there

@pllim Sorry about that but I am lost kind of lost on what am I supposed to do next. I don’t have much experience with rebase, something goes wrong whenever I try it, so it would be great if you perform the rebase you can or guide me on how can I rebase. Thanks

@Mubin17 , please try the following:

1. First make a backup branch, in case things go wrong.
2. git fetch upstream main
3. Then on this branch, do git rebase upstream/main
4. If there is any conflict, it will guide you through conflict resolution, so read the prompts.
5. If no conflicts or after they are resolved, do a git log. You should see only your commits without merge commit in between.
6. If you are still unsure, do a git diff HEAD~2 (since right now you have 2 real commits) and you should see only your changes.

When you are confident it is correctly done, do git push origin Mubin17/14840Issue -f

Good luck!

@pllim Thanks for the help!

I notice this pull request has two commits: the first fixes the rule violations in astropy.io, the second edits .ruff.toml. If pre-commit is set up then it would be simpler to first edit .ruff.toml and then to make the commit without manually editing any other files. If Ruff can autofix the rule (it can autofix C408) then the Ruff pre-commit hook automatically fixes the rule violations and the Black hook automatically fixes any code style issues (if any are needed).

I notice this pull request has two commits: the first fixes the rule violations in astropy.io, the second edits .ruff.toml. If pre-commit is set up then it would be simpler to first edit .ruff.toml and then to make the commit without manually editing any other files. If Ruff can autofix the rule (it can autofix C408) then the Ruff pre-commit hook automatically fixes the rule violations and the Black hook automatically fixes any code style issues (if any are needed).

Thanks @eerovaher for the suggestion! I feel like editing the .ruff.toml file first and then committing the changes without manual file editing sounds like a much smoother workflow. I'll definitely keep this in mind when I tackle Ruff rule-C408 in other Astropy sub-packages.

Just a quick question: Do you think the current pull request is ready to be merged, or if there are any additional changes or considerations you would like me to address? Thanks again!

I suggest squashing the commits.

I have an additional recommendation about commit messages. GitHub knows which commits belong to which pull requests regardless of the commit messages, and linking pull requests to issues does not depend on the commit messages either. Including GitHub issues in commit messages therefore provides little benefit to GitHub users. Furthermore, issues and pull requests are a GitHub feature, not a Git feature, so including issue numbers in commit messages usually doesn't help anyone using Git either, although there are some exceptions to this. Replacing dict() calls with dictionary comprehensions does not depend on GitHub at all, so this is not one of the exceptions where adding an issue number to the commit message would be helpful.

@eerovaher I've squashed all the commits into one and have updated the commit messages to an appropriate one.

@eerovaher I have made the requested changes, can you please review them? Let me know if anything else needs changes. Thanks

@nstarman Does this PR need any additional changes? If not can it be merged?

I think we’re good to go. IMO this is a clear and positive change. Thanks @Mubin17 for the PR!