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

Made set_attenuation_coefficients private #243

Merged
merged 22 commits into from
Nov 8, 2024

Conversation

Abinash-bit
Copy link
Contributor

@Abinash-bit Abinash-bit commented Nov 1, 2024

Summary

This pull request resolves conflicts and merges the latest changes from the main branch into the feature-branch. The changes include:

  • Integrated the logic of set_attenuation_coefficients directly into the fnrgf function.
  • Updated the fnrgf function to accept range_mean, range_std, and cutoff parameters.
  • Resolved conflicts in radial.py and test_radial.py.

Changes Made

  • Merged the latest changes from the main branch.
  • Resolved conflicts in the fnrgf function.
  • Updated the test cases in test_radial.py to reflect the changes.

Testing

  • All tests in test_radial.py have been updated and pass successfully.

Related Issues

Checklist

  • Code follows the project's coding standards.
  • Documentation has been updated.
  • Tests have been added or updated to cover the changes.
  • All tests pass successfully.

Reviewers

@Abinash-bit
Copy link
Contributor Author

Hey @nabobalis please have a look.

@nabobalis
Copy link
Contributor

So this replaces the original pull request?

sunkit_image/radial.py Outdated Show resolved Hide resolved
sunkit_image/radial.py Outdated Show resolved Hide resolved
sunkit_image/radial.py Outdated Show resolved Hide resolved
@nabobalis nabobalis changed the title Resolved conflicts and merged main branch Made set_attenuation_coefficients private Nov 4, 2024
@nabobalis
Copy link
Contributor

This will need a breaking changelog entry as well to round off the PR.

Copy link
Contributor

@nabobalis nabobalis left a comment

Choose a reason for hiding this comment

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

Just needs the changelog tweaking but it LGTM

@nabobalis
Copy link
Contributor

Sphinx-Gallery successfully executed 10 out of 11 files subselected by:
WARNING: Here is a summary of the problems encountered when running the examples:

Unexpected failing examples (1):

    ../examples/radial_gradient_filters.py failed leaving traceback:

    Traceback (most recent call last):
      File "/home/runner/work/sunkit-image/sunkit-image/examples/radial_gradient_filters.py", line 51, in <module>
        attenuation_coefficients = radial.set_attenuation_coefficients(order)
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AttributeError: module 'sunkit_image.radial' has no attribute 'set_attenuation_coefficients'. Did you mean: '_set_attenuation_coefficients'?

Looks like one of the examples needs to be updated.

Abinash-bit and others added 3 commits November 7, 2024 01:20
This merge incorporates the latest changes from the remote feature-branch, resolving conflicts in tests/test_radial.py.# Please enter a commit message to explain why this merge is necessary,
@nabobalis
Copy link
Contributor

Now just the changelog and the precommit left.

@Abinash-bit
Copy link
Contributor Author

Screenshot 2024-11-08 112503

i can not get the error , like where is the actual error.

@nabobalis
Copy link
Contributor

I think it would be easier to run the pre-commit locally and have it fix the issues for you instead of doing it manually.

@Abinash-bit
Copy link
Contributor Author

Hey @nabobalis i think all the checks have passed please review.

@Abinash-bit
Copy link
Contributor Author

Hey is this the correct way , apart from moving to utils ??

@Abinash-bit
Copy link
Contributor Author

Hey @nabobalis i just pressed commit suggestions , now in the latest commit some test got failed , please help me to uncommit the latest commit.

@nabobalis
Copy link
Contributor

Hey @nabobalis i just pressed commit suggestions , now in the latest commit some test got failed , please help me to uncommit the latest commit.

We don't need to uncommit, we just need to fix it in another commit.

@nabobalis
Copy link
Contributor

Thank you very much @Abinash-bit

@nabobalis nabobalis merged commit 24bb714 into sunpy:main Nov 8, 2024
23 checks passed
@Abinash-bit
Copy link
Contributor Author

Thank you so much for your guidance in every steps @nabobalis .

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.

2 participants