search

SciTools / cf-units

Units of measure as required by the Climate and Forecast (CF) Metadata Conventions
https://cf-units.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
64 stars 46 forks source link

Make antlr optional #423

Open ocefpaf opened 4 months ago

ocefpaf commented 4 months ago

Closes https://github.com/SciTools/cf-units/issues/313

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

ocefpaf commented 4 months ago

I wanted to add a test without antlr-python-runtime but I have no idea how to do that in the complex tox/conda-lock/extra env file setup here. If someone wants to add that note that we would need to call pytest with pytest cf_units/tests to avoid collecting import errors, or use -continue-on-collection-errors to continue.

I can attest that this works. All the functionalities but latex parsing are available and the import error is informative enough, if/when someone tries to use latex.

pp-mo commented 4 days ago

NOTE: update thoughts Despite the exciting possibilities of #445 and a possible stepping-stone to #446, that doesn't make a reason to not do this first -- it's still a useful + a simple first step.

pp-mo commented 4 days ago

Status update 2024/9/13 : this is a good change, but can't just resolve + merge. Want to incorporate changes to the env ymls, and then re-resolve the lockfiles

ocefpaf commented 4 days ago

it's still a useful + a simple first step.

That would help a lot while we wait for something better.

but can't just resolve + merge. Want to incorporate changes to the env ymls, and then re-resolve the lockfiles

I confess that the CI setup here grow both in complexity and size that makes me not want to mess with them. Even more so in a case for an optional dependency, which can increase the number of files here. Is that something that is on your radar or as you expecting me, as the PR author, to do it? If the latter, I'd love some guidance before I tackle this.