Clippy pendantic linting

[dahlend]

Hi there!

Thanks for the crate!

I was looking at using some of your crates for dealing with FITs files for a project, but there is some missing support for a few map projections.

I write orbit propagation code to identify asteroids/comets in images, see:
https://github.com/dahlend/kete

I wanted to see if you were open to me providing some pull requests which would add tests for SIP, and add TPV?

I know this pull request looks scary, but its not! It doesn't change any math or the API. I know it looks enormous, but is mostly just applying clippy pendantic settings to this crate. If you dont like it I am happy to switch back, this is mostly just me reaching out to see how open you were to new pull requests.

See: https://doc.rust-lang.org/clippy/

Changes:

• Reflow 2 spaces to 4 spaces as is rust style guide (happy to change back).
• Adds Debug, Clone, and Copy to all structs where possible.
• Adds #[must_use] to all functions which return values.
• Adds a .gitignore
• Adds a long list of rust lint settings along with rust clippy settings.
• Fixes all suggestions which rust clippy provided.
• Switch custom HALF_PI to f64::FRAC_PI_2

[fxpineau]

Hi @dahlend,

I was on vacation for the last two weeks and I missed this message.

I wanted to see if you were open to me providing some pull requests which would add tests for SIP, and add TPV?

Yes, sure.
Indeed, I think we have no usage and test cases for SIP so far. Thus it is probably incomplete/buggy. @bmatthieu3 ?

Reflow 2 spaces to 4 spaces as is rust style guide (happy to change back).

Yes, please, change back (using --config tab_spaces=2 in rustfmt): this is the only voluntary deviation from the standard.

Thank you very much for sharing your expertise and contribution.

[bmatthieu3]

Hi @dahlend,

This crate is used in aladin lite, a web tool to display FITS images on top of HiPS large-scale surveys, something like an openstreetmap of the sky. It would be very beneficial for it to support correctly SIP and TPV as well, so I am very glad if you want to contribute on these.

Thank you very much for this

[dahlend]

Thanks for getting back to me!

I will clean this up a bit and mark it as ready for review.

Not in this PR, but I believe I have SIP working (number of minor bugs and got inversion working reliably). Still working on TPV, unfortunately it adds a lot of new parameters (40 per axis)...

[dahlend]

Hi!

I switch this back to 2 spaces, the original changes still stay:

Adds Debug, Clone, and Copy to all structs where possible.
Adds #[must_use] to all functions which return values.
Adds a .gitignore
Adds a long list of rust lint settings along with rust clippy settings.
Fixes all suggestions which rust clippy provided.
Switch custom HALF_PI to f64::FRAC_PI_2

After this I have completed a fully tested SIP and TPV implementations, along with adding the missing projections. I will need to break those up into a series of pull requests across at least 2 repositories.

Getting the inversions right for SIP and TPV took time, I match wcslib to essentially numerical precision in all of my tests.

[bmatthieu3]

Hi @dahlend

Many thanks for these changes. I think I do not have so much to say about it, these are great.
Would you mind to add your changes in the CHANGELOG.md ? especially to keep track of the must_use change which I have seen can break code for crates using deny(warnings)

[bmatthieu3]

Hi @dahlend, it is been a long time since you made the PR. We would like to merge it as soon as possible. We have only 3 things that we would like to investigate more and we are not ready to merge it for now. Those are:

• the must_use decorator everywhere
• the linting rules in cargo.toml

Please also add an entry to the changelog.md with your changes

Appart from that this PR can be merged. Thank you very much.

[bmatthieu3]

Can you remove that ? We prefer to not have it for the moment because we will take some time to know more about linting rules that we want (or not).

[bmatthieu3]

Could you remove all these must_use decorator ? In many cases I would not say not using the result would automatically lead to a bug. Also, lots of CI define warnings as errors (RUSTFLAGS="-D warnings" cargo build) and this may break the CI whereas no particular bug is really there (e.g. a call that is not necessary that is why its result is not used).

[bmatthieu3]

@dahlend. Another point. I fixed some bugs regarding the SIP support #5 . It may be that you found other potential problems so feel free to open another PR for that. We are also very interested into your TPV impl that you mentioned earlier. Many thanks