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

remove angstrom #14

Closed
xzackli opened this issue Dec 3, 2019 · 4 comments
Closed

remove angstrom #14

xzackli opened this issue Dec 3, 2019 · 4 comments

Comments

@xzackli
Copy link
Contributor

xzackli commented Dec 3, 2019

Due to PainterQubits/Unitful.jl#271 there is now a duplication in Unitful and UnitfulAstro for Angstrom. Since nobody will be using UnitfulAstro alone, just remove all references to it in this package?

(Thanks for the great package!)

@giordano
Copy link
Member

giordano commented Dec 3, 2019

Yes, that's a good idea. Would you mind opening a pull request yourself? 🙂

@xzackli
Copy link
Contributor Author

xzackli commented Dec 4, 2019

Sure!

giordano added a commit that referenced this issue Jan 3, 2020
* removed every mention of angstrom as it is now redundant, fixes issue #14

* reverted changes to NEWS.md and tests, bumped compat version of Unitful to 0.18.0

* fixed test by including base Unitful

* Unitful angstrom is implemented as a rational number type multiplied by the nanometer, so we should write tests with the rational number type

* promotion to Float64 in angstrom test

* Use uppor bounds in compat section

* Use isapprox for angstrom test

* Restore other test

Co-authored-by: Mosè Giordano <[email protected]>
@briochemc
Copy link

Tag a new release with registrator?

@giordano
Copy link
Member

Done, thanks for the reminder!

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

3 participants