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

WIP: split core functionality into UnitfulBase #428

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Gregstrq
Copy link

@Gregstrq Gregstrq commented Mar 6, 2021

This PR adresses issue #424 and issue #359.

  • I have moved all the files except "pkgdefaults.jl" and "dates.jl" into a subdirectory package UnitfulBase.

  • I also moved the definitions of radian, steradian and degrees into UnitfulBase. The reason for that was the extension of the function mod2pi which uses degrees. I have decided to move degrees into UnitfulBase instead of keeping mod2pi extension in Unitful. Since radian and steradian are dimensionless and thematically connected to degrees, I have moved them as well.

  • Unitful is now a registered extension of UnitfulBase.

  • I have edited the tests so they work with the splitted UnitfulBase. I have run the tests on my computer: they run successfully with the exception of the single test for issue @u_str macro hygiene? #272 (see these lines). The problem is that if @u_str is imported without at least importing Unitful, Unitful is not registered as an extension of UnitfulBase.

  • Since UnitfulBase is not a registered package yet, the only way to add it to Unitful project environment was to dev it. The information about the path to UnitfulBase was stored only in Manifest.toml, which I did not commit. As a result, the tests might be broken in CI for now. If this PR will ever get merged, the problem will disappear once UnitfulBase is registered as a package.

I did not touch the docs yet. I would like to hear your opinion about the contents of this PR first.

@Gregstrq
Copy link
Author

I understand that the idea of the splitting of the core functionality is an arguable thing.

I created this PR as an experiment to poke around and to see what we get.
That said, I am eager to work on any feedback from the maintainers.
(May be, a good approach is to create an experimental branch and push this PR there instead of master.)

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

Successfully merging this pull request may close these issues.

1 participant