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

changed all instances of "missing_axis" to "missing_axes" #157

Merged
merged 6 commits into from
May 14, 2019

Conversation

gnshafreak
Copy link

@gnshafreak gnshafreak commented Apr 10, 2019

Description

Fixes #130

@DanRyanIrish
Copy link
Member

ping @Cadair. Your dream come true ;)

@Cadair Cadair self-requested a review April 10, 2019 20:39
@Cadair
Copy link
Member

Cadair commented Apr 10, 2019

Thanks @gnshafreak I will take a look later 😄

@DanRyanIrish
Copy link
Member

DanRyanIrish commented Apr 12, 2019

Hi @gnshafreak. This looks great. So that we don't break the API, you also add a missing_axis property to NDCubeBase? I should raise a DeprecationWarning saying that missing_axes has replaced missing_axis and ```missing_axis will no longer be supported in 2.0? So it would be something like this:

@property
def missing_axis(self):
    # Issue deprecation warning here with message "missing_axis has been deprecated by missing_axes and will no longer be supported after ndcube 2.0".
    return self.missing_axes

@ghost
Copy link

ghost commented Apr 25, 2019

Thanks for the pull request @gnshafreak!

I am a bot that checks pull requests for milestones and changelog entries. If you have any questions about what I am saying, please ask!
I have the following to report on this pull request:

  • This pull request does not have a milestone assigned to it. Only maintainers can change this, so you don't need to worry about it. 😄

If there are any issues with this message, please report them here.

@DanRyanIrish
Copy link
Member

Hi @gnshafreak. This deprecation warning looks good! I think this may be ready to merge. Depending on the reason for the circleci test failure.

@Cadair @nabobalis pip seems to be failing to install something for the circleci tests. Is this a known issue with sunpy as well? https://circleci.com/gh/sunpy/ndcube/129?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@nabobalis
Copy link
Contributor

Yes, that is an upstream issue that should be fixed in the next pip release.

@DanRyanIrish
Copy link
Member

DanRyanIrish commented Apr 29, 2019

Thanks @nabobalis! In that case, I think this PR is ready to merge! Any objects @Cadair?

CHANGELOG.rst Outdated
@@ -10,6 +10,7 @@ API-Breaking Changes
New version:
``crop_by_extra_coord(coord_name, min_coord_value, max_coord_value)``.
[#142]
- Changed all instances of "missing_axis" to "missing_axes"
Copy link
Member

Choose a reason for hiding this comment

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

Delete this line.

@DanRyanIrish
Copy link
Member

Add a file in the changelog/ directory called 157.feature.rst
Add the changelog entry in there.

@DanRyanIrish DanRyanIrish merged commit 95b60b1 into sunpy:master May 14, 2019
@DanRyanIrish DanRyanIrish added this to the 1.2 milestone Jun 3, 2019
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.

The missing_axis argument to NDCube and attribute name is a typo
4 participants