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

Constructors #26

Merged
merged 4 commits into from
Nov 8, 2019
Merged

Constructors #26

merged 4 commits into from
Nov 8, 2019

Conversation

mileslucas
Copy link
Member

@mileslucas mileslucas commented Oct 21, 2019

This pull request adds some extra constructors. This depends on #25 to be merged first, so I will leave it as a draft until then.

  1. Extra constructors
    I've added some constructors that follow this syntax
c1 = ICRSCoords(...)
c2 = GalCoords(c1)

which can also allow the convenient piping syntax

c1 = ICRSCoords(...)
c2 = c1 |> GalCoords

I know the piping syntax is somewhat controversial, but I think for these type conversions it is a really clean way to signify the conversion. I really like its usage in Unitful, for example.

So far I've added the appropriate constructors for each type and added in tests to check they are converted in the same way as Base.convert does. As it stands, there might be interest in adding some sort of metaprogramming to write these constructors since they are somewhat consistent across all types (although not perfect because of the equinox parameter for FK5).

  1. Version bump
    I've incremented version to 0.5.0-dev

@codecov-io
Copy link

codecov-io commented Oct 22, 2019

Codecov Report

Merging #26 into master will decrease coverage by 3.29%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #26     +/-   ##
=========================================
- Coverage   89.65%   86.36%   -3.3%     
=========================================
  Files           1        2      +1     
  Lines          58       66      +8     
=========================================
+ Hits           52       57      +5     
- Misses          6        9      +3
Impacted Files Coverage Δ
src/SkyCoords.jl 91.3% <ø> (+1.64%) ⬆️
src/types.jl 75% <75%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac24a37...73c9a3e. Read the comment docs.

@mileslucas mileslucas marked this pull request as ready for review November 5, 2019 21:46
@mileslucas mileslucas requested a review from giordano November 5, 2019 21:47
@@ -1,7 +1,7 @@
name = "SkyCoords"
uuid = "fc659fc5-75a3-5475-a2ea-3da92c065361"
authors = "Kyle Barbary, Mosé Giordano, and contributors"
version = "0.4.0"
version = "0.5.0-DEV"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can directly use 0.5.0 here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have plans for another PR after this one to allow string construction, which is mostly written already so it could make sense to push that one as 0.5.0

@giordano giordano merged commit 8a56a00 into JuliaAstro:master Nov 8, 2019
@mileslucas mileslucas deleted the constructors branch November 12, 2019 21:48
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.

3 participants