Add a pre-commit hook for this tool

[nstarman]

Looks very useful! A pre-commit tool to run this as a CI check would be lovely.

[nstarman]

astropy/astropy#15257 for a discussion on this in Astropy

[asmeurer]

I don't really have time to work on this right now. This was mainly a tool that I cooked up to help me solve a problem I had at the time. But if you or someone else wants to work to improve it I'm happy to give people push access.

[Saransh-cpp]

Hi @asmeurer! Thanks for this amazing tool! I just bumped into this issue and I have been working on something similar as a side project -

My fork, rmstar, has the latest packaging infrastructure and I am working to create a pre-commit hook out of the same. Right now the library works with hatch as a build-backend (hatch-vcs for versioning) + no setup.py, setup.cfg, versioneer, ... stuff, but the pre-commit stuff is a bit wobbly. I'll probably make the first release by the end of this semester.

I would be more than happy to create PRs here once I get the pre-commit stuff working on my fork, or I would also be more than happy to maintain the fork separately. Please let me know how I should work on this. Thanks! :)

---

Notes from the README of rmstar -

Warning

This is not ready to be used as a pre-commit hook yet. The CLI works fine, and the infrastructure is up-to-date. This should be ready as a pre-commit hook by the end of this year!

Note

rmstar is an actively maintained fork of the original removestar (which is now not actively maintained). All the credits for the original code and logic goes to Aaron Meurer (or asmeurer) and the contributors of removestar. This repository keeps the original commits intact and builds on top of them. rmstar also comes as a pre-commit hook for ease of use (which was the original motivation behind this fork).

[asmeurer]

Great. There's no need to fork to a separate project, though. Like I said, I'm happy to give people access here if you want to continue work on removestar

[Saransh-cpp]

Yes, I would love that! I can start pushing the changes that are actually ready, including pyproject.toml, hatch, and hatch-vcs migration.

[Saransh-cpp]

I got the pre-commit working :)

Adding the following to astropy's v5.3.2 codebase -

  - repo: https://github.com/Saransh-cpp/rmstar
    rev: f7c340ad7d8cc9496aaadbd88ae27b7e41dfcfd1  # Haven't made a release
    hooks:
      - id: rmstar
        args: [] # See removestar's docs for all args (-i edits file in-place)
        additional_dependencies: # The libraries or packages your code imports
          - . # Should be . if running inside a library (to install the library itself in the environment)
          - sphinx_astropy

gives me the following output -

rmstar...................................................................Failed
- hook id: rmstar
- duration: 4.4s
- exit code: 1

--- original/astropy/modeling/models.py
+++ fixed/astropy/modeling/models.py
@@ -6,15 +6,15 @@
 
 from . import math_functions as math
 from .core import custom_model, fix_inputs, hide_inverse
-from .functional_models import *
-from .mappings import *
-from .physical_models import *
-from .polynomial import *
-from .powerlaws import *
-from .projections import *
-from .rotations import *
-from .spline import *
-from .tabular import *
+from .functional_models import (AiryDisk2D, ArcCosine1D, ArcSine1D, ArcTangent1D, Box1D, Box2D,
+                                Const1D, Const2D, Cosine1D, Disk2D, Ellipse2D, Gaussian1D,
+                                Gaussian2D, KingProjectedAnalytic1D, Linear1D, Lorentz1D, Moffat1D,
+                                Moffat2D, RickerWavelet1D, RickerWavelet2D, Sersic1D, Sersic2D,
+                                Sine1D, Tangent1D, Trapezoid1D, TrapezoidDisk2D, Voigt1D)
+from .physical_models import NFW
+from .polynomial import (Chebyshev1D, Chebyshev2D, Hermite1D, Hermite2D, Legendre1D, Legendre2D,
+                         Polynomial1D, Polynomial2D)
+from .powerlaws import PowerLaw1D
 
 # Attach a docstring explaining constraints to all models which support them.
 # Note: add new models to this list

  warnings.warn(
Warning: docs/conf.py: The removed star import statement for 'sphinx_astropy.conf.v1' had an inline comment which may not make sense without the import
--- original/docs/conf.py
+++ fixed/docs/conf.py
@@ -60,7 +60,7 @@
     print('Please install the "docs" requirements.')
     sys.exit(1)
 
-from sphinx_astropy.conf.v1 import *  # noqa: E402
+# noqa: E402
 from sphinx_astropy.conf.v1 import (  # noqa: E402
     exclude_patterns,
     extensions,

The output does look good. Adding the -i argument also works and edits the files in place!

astropy/astropy#15258 cleaned up the import * problem in modeling/models.py, but it did not touch the from sphinx_astropy.conf.v1 import * line, which rmstar points out above. @nstarman, is the removal suggestion correct, or was this import intentionally not removed?

PS: Please let me know how should I push the changes upstream.

[nstarman]

@Saransh-cpp this looks very good! The sphinx_astropy.conf.v1 import * removal is partially correct in that we should probably replace it for explicit imports, though the straight removal is incorrect. Of course a human review will flag and correct this. I'm thinking that there should be a check for noqa: F403, to turn off importstar on specific lines.

[nstarman]

@Saransh-cpp, you can open a PR on Astropy adding removestar to pre-commit. Pre-commit and the CI will do its work! Thanks for putting the time into this project

[asmeurer]

Sent you both invites to be collaborators on this repo.

[kieran-ryan]

Hi all, just saw this issue; I created a pre-commit hook for removestar (removestar-pre-commit) a few months back for personal use if useful - but looks like changes are nearly there

Awesome project by the way ✌️

[Saransh-cpp]

Sent you both invites to be collaborators on this repo.

Thanks! I'll still create PRs and merge them on my own just so the changes are easily visible.

I'm thinking that there should be a check for noqa: F403

There is already one, according to the docs here - https://github.com/asmeurer/removestar#whitelisting-star-imports

you can open a PR on Astropy adding removestar to pre-commit

Great! Will do once I push all the changes upstream!

Hi all, just saw this issue; I created a pre-commit hook for removestar (removestar-pre-commit) a few months back for personal use if useful - but looks like changes are nearly there

This looks great! I'll take a look at the config!

[asmeurer]

If someone wants to take a look at the existing open PRs that would be great too. I've been neglecting them, but I think at least #31 is probably ready to be merged.

[Saransh-cpp]

Everything works now - https://results.pre-commit.ci/run/github/2081289/1693857593.zi7J0ShNQN-5w65zCDEgBA! @nstarman @kieran-ryan could you please take a look?

@asmeurer could you please configure the trusted publisher thing for this repository? I will then make a release (if everything looks right) on GitHub, triggering cd.yml and creating a new release on PyPI.

Thanks!

[nstarman]

The PR in Astropy looks good. We'll have to add F403 to the noqa instead of removing the star import, but that's alright, removestar is for finding and suggesting fixes for human review. 👍

[asmeurer]

Just give me your PyPI username and I'll add you there.

[Saransh-cpp]

Great! That would be Saransh-cpp - thanks!

[Saransh-cpp]

Thanks for the invite, @asmeurer! Unfortunately, I cannot access the "Manage" part (and subsequently the "Publishing" part) of the project. Would it be possible for you to bump my role in PyPI? Thanks again!

[Saransh-cpp]

Made a new release and everything works well so far!

Closing this, thanks for the discussion here!