Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obligatory "Josh just wrote the docs and now has to complain about everything" issue #27

Closed
3 of 4 tasks
Yoshanuikabundi opened this issue May 5, 2022 · 4 comments
Closed
3 of 4 tasks

Comments

@Yoshanuikabundi
Copy link
Contributor

Yoshanuikabundi commented May 5, 2022

I'm putting this in an issue so that it doesn't get forgotten (by me if no-one else) when the docs PR is merged, and I'm putting it all in one issue to avoid spam. Here are my thoughts:

  • 1. to_openmm would make a great method for Quantity. This would save users an import when going in that direction. For bonus points, we could define it as a method on Unit as well and make the free function polymorphic over both
  • 2. having all the units be under openff.units.unit while the three big classes are under openff.units.units is confusing. If API breaks are OK, maybe we could rename openff.units.units to openff.units._pint and re-export Quantity, Unit, and Measurement from openff.units (either lazily or not) (Fixed in Add documentation #28)
  • 3. It might be prudent to pin to a major version of Pint in each release. Our API is mostly inherited from theirs, so we can't really claim not to break things without a pin. In particular, our docs change a lot based on which version of Pint is installed in the RTD environment when its built. If we do this, it would be great to pick at least 0.18.0, as type hints were added to Quantity in that version.
  • 4. It would be amazing to be able to specify the shape and units of a Quantity[Array] in type hints, does anyone know if that's possible? I know they're both part of the value and not the type but the two concepts are kinda fungible in Python. I vaguely remember that people somewhere somewhen were working on it.
@mattwthompson
Copy link
Member

It would be amazing to be able to specify the shape and units of a Quantity[Array] in type hints, does anyone know if that's possible?

My best guess is that it's possible-but-difficult with PEP 646, but that's 3.11+ / I haven't looked for a backport.

@mattwthompson
Copy link
Member

Oh, and for the record, I see four great suggestions and no complaints!

@mattwthompson mattwthompson mentioned this issue Jun 2, 2022
5 tasks
@mattwthompson
Copy link
Member

After #34 I think 1-3 are now incorporated and 4 probably won't happen for a while. Would you be willing to copy+paste that into a new issue and close this one?

@Yoshanuikabundi
Copy link
Contributor Author

@mattwthompson Too easy :)

Superseded by #35

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

No branches or pull requests

2 participants