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

Removed `sunpy.sun.sun.solar_cycle_number` #3150

Merged
merged 1 commit into from May 29, 2019

Conversation

Projects
None yet
3 participants
@ayshih
Copy link
Contributor

commented May 29, 2019

This function has been broken since its inception in 2011. This was noted as broken in 2013 and re-noted as broken in 2014, but no one ever did anything about it. Now I am. Bye bye!

To be explicit, it's fundamentally broken because:

  • The solar-cycle numbers that it returns are nonsensical; they increase by 1 every year and wrap every 28 years.
  • Even if the function were fixed to give something close to reasonable (by changing the modulo to a division by a nominal solar-cycle period), we can't possibly return something that's uncontroversial, so why even try?

@ayshih ayshih added this to the 1.0 milestone May 29, 2019

@ayshih ayshih requested a review from Cadair May 29, 2019

@Cadair

Cadair approved these changes May 29, 2019

@Cadair

This comment has been minimized.

Copy link
Member

commented May 29, 2019

🔥 🔥 🔥

@nabobalis nabobalis merged commit ebf92be into sunpy:master May 29, 2019

16 checks passed

ci/circleci: 32bit Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-37 Your tests passed on CircleCI!
Details
ci/circleci: figure-tests-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
ci/circleci: pip-install Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing c597795...79aa192
Details
codecov/project 90.29% (-0.01%) compared to c597795
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy Build #20190529.17 succeeded
Details
sunpy.sunpy (Linux_36_Conda_offline) Linux_36_Conda_offline succeeded
Details
sunpy.sunpy (Linux_36_offline) Linux_36_offline succeeded
Details
sunpy.sunpy (Linux_37_online) Linux_37_online succeeded
Details
sunpy.sunpy (Windows_36_offline) Windows_36_offline succeeded
Details
sunpy.sunpy (macOS_37_offline) macOS_37_offline succeeded
Details

@ayshih ayshih deleted the ayshih:scn branch May 29, 2019

nabobalis added a commit to nabobalis/sunpy that referenced this pull request May 30, 2019

Merge pull request sunpy#3150 from ayshih/scn
Removed `sunpy.sun.sun.solar_cycle_number`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.