search

acl-org / aclpubcheck

Tools for checking ACL paper submissions
MIT License
598 stars 47 forks source link

Adds aclpubcheck command line interface #40

Closed tnaumann closed 8 months ago

tnaumann commented 2 years ago

As is, this creates the aclpubcheck command as a shim that effectively serves as an alias for python3 formatchecker.py in the documentation.

tnaumann commented 2 years ago

Two questions arise from the above implementation, especially as they relate to #39:

  1. I'm assuming that we want a single entry point for authors to run the tool. Does python3 formatchecker.py --paper_type PAPER_TYPE PAPER_NAME.pdf currently check everything we want them to check?
  2. Do you want me to upload to PyPI so installation simply becomes pip install aclpubcheck rather than installing as below? If so, is there an preferred account to upload with? I'm happy to use my own, but might make more sense to set up one for acl-org and upload as that account.
kylebgorman commented 9 months ago

+1. This really should be given priority if ACs are recommending that authors use this tool. Happy to review further if needed.

ryancotterell commented 9 months ago

It's a good idea. Do you have any bandwidth to take it on?

kylebgorman commented 9 months ago

If this is destined for PyPI it ought to follow PEP 518 and use pyproject.toml. (You can still also have a setup.py, and sometimes you need both, but I don't think that's the case here.) If you have a working setup.py-based installation committed to the repo, I will volunteer to migrate it to the PEP 518-conformant installation at a later point.

xhluca commented 9 months ago

@kylebgorman I created a PR #66 that solely uses setup.py; it currently works with pip install git+https://github.com/xhluca/aclpubcheck (xhluca --> acl-org if merged). Would the approach suggested not be compliant with PEP-518?

crux82 commented 9 months ago

Another question: will it work with the online versions? https://github.com/acl-org/aclpubcheck?tab=readme-ov-file#online-versions They are crucial to help people that are not able to install the software.

kylebgorman commented 9 months ago

Another question: will it work with the online versions? https://github.com/acl-org/aclpubcheck?tab=readme-ov-file#online-versions They are crucial to help people that are not able to install the software.

It's hard for me to imagine there's even a single ACL author who can't or doesn't know how to run a modern Python program. But nothing should work differently on Colab vs. locally.

kylebgorman commented 9 months ago

@kylebgorman I created a PR #66 that solely uses setup.py; it currently works with pip install git+https://github.com/xhluca/aclpubcheck (xhluca --> acl-org if merged). Would the approach suggested not be compliant with PEP-518?

Whether or not you have a setup.py, PEP 518 requires that you have a pyproject.toml file which contains information about things like dependencies. While I have one rather complicated project that requires both, the majority of libraries only need the latter. But it's straightforward to migrate from setup.py to pyproject.toml most of the time, which is why I'm volunteering.

xhluca commented 9 months ago

@kylebgorman thanks for the clarification. It definitely makes sense to have the toml for cases where users don't have setuptools installed. I agree it's a good idea to have a separate PR once setup.py is updated on main; this probably would make reviewing simpler.

@crux82 if you are referring to the PR I created yesterday (#66), yes it will be compatible with the colab with minor changes. I'm happy to update the current colab and include the ipynb as part of the PR (I can update the existing url to link to the new ipynb, which colab will automatically render into a colab notebook). If you are referring to this PR, i think there are changes necessary to the colab notebook but I don't have write permission to edit it.

crux82 commented 8 months ago

It should be solved in https://github.com/acl-org/aclpubcheck/pull/66

Thank you all for the support!!!