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

Add a support to write NDCube data into FITS file #179

Closed
wants to merge 27 commits into from

Conversation

yashrsharma44
Copy link
Member

@yashrsharma44 yashrsharma44 commented Jun 4, 2019

This PR addresses issue #111.
This is a continuation of the PR #154.
All the relevant discussions have taken place there.

Tasks before this is merged -

  • Tests for the new method in ndcube.py
  • Tests for the ndcubeio.py
  • Docs and examples
  • Squash the commits into one

Ping @DanRyanIrish

1. Used mixin class to implement the write method
2. This would ensure that we can add further read and write functions for different formats
3. Only three types of uncertainty have been added.
1. Added the NDCubeIOMixin as a mixin class for ndcube
2. Added a writer function
3. Registered the writer function
4. Added a cross-check for ensuring the fits convention is followed
1. Changed the uncertainty defaults
2. Now any uncertainty along with any uncertainty data type would be accepted.
1. Changed the mixin class design
2. Removed the constructor
1. Changed the comments of the flattened dictionary
2. Corrected some typo for fits_checker
3. Changed the name of the `to_hdu` to `to_hdulist`
4. Changed the name of the ImageHDU list
5. Used `isinstance` to conform to numpy mask
6. Changed the LABEL for extra coords
7. Shifted the `fits_ndcube_writer`
1. Moved the label for missing and extra_coords inside the function
2. Corrected the `self._mask` parameter
1. Changed to the correct import order
2. Added the function name to the list of public objects
@ghost
Copy link

ghost commented Jun 4, 2019

Thanks for the pull request @yashrsharma44!

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. 😄
  • I didn't detect a changelog file in this pull request. Please add a changelog file to the changelog/ directory following the instructions in the changelog README.

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

@pep8speaks
Copy link

pep8speaks commented Jun 5, 2019

Hello @yashrsharma44! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 67:5: E303 too many blank lines (3)
Line 74:5: E303 too many blank lines (2)
Line 140:9: E265 block comment should start with '# '
Line 142:9: E265 block comment should start with '# '
Line 152:9: E265 block comment should start with '# '
Line 154:9: E265 block comment should start with '# '
Line 165:9: E265 block comment should start with '# '
Line 167:9: E265 block comment should start with '# '
Line 177:9: E265 block comment should start with '# '
Line 179:9: E265 block comment should start with '# '
Line 216:9: E265 block comment should start with '# '

Line 632:71: W292 no newline at end of file

Line 109:13: E129 visually indented line with same indent as next logical line
Line 157:89: W292 no newline at end of file

Line 5:5: E116 unexpected indentation (comment)
Line 6:5: E116 unexpected indentation (comment)
Line 7:5: E116 unexpected indentation (comment)
Line 9:5: E116 unexpected indentation (comment)
Line 11:5: E116 unexpected indentation (comment)
Line 13:5: E116 unexpected indentation (comment)
Line 15:5: E116 unexpected indentation (comment)
Line 57:1: E302 expected 2 blank lines, found 1
Line 58:101: E501 line too long (112 > 100 characters)
Line 58:111: E231 missing whitespace after ','
Line 59:35: E231 missing whitespace after ','
Line 59:54: E231 missing whitespace after ','
Line 59:78: E231 missing whitespace after ','
Line 59:84: E231 missing whitespace after ','
Line 59:91: E231 missing whitespace after ','
Line 59:99: E231 missing whitespace after ','
Line 59:101: E501 line too long (150 > 100 characters)
Line 59:110: E231 missing whitespace after ','
Line 59:122: E231 missing whitespace after ','
Line 59:140: E231 missing whitespace after ','
Line 62:101: E501 line too long (133 > 100 characters)
Line 100:1: E302 expected 2 blank lines, found 1
Line 101:50: E231 missing whitespace after ','
Line 101:78: E231 missing whitespace after ','
Line 101:86: E231 missing whitespace after ','
Line 101:99: E231 missing whitespace after ','
Line 101:101: E501 line too long (109 > 100 characters)
Line 101:102: E231 missing whitespace after ','
Line 102:5: E124 closing bracket does not match visual indentation
Line 121:50: E231 missing whitespace after ','
Line 121:78: E231 missing whitespace after ','
Line 121:86: E231 missing whitespace after ','
Line 121:99: E231 missing whitespace after ','
Line 121:101: E501 line too long (118 > 100 characters)
Line 121:109: E231 missing whitespace after ','
Line 122:5: E124 closing bracket does not match visual indentation
Line 141:43: E231 missing whitespace after ','
Line 141:71: E231 missing whitespace after ','
Line 141:79: E231 missing whitespace after ','
Line 141:92: E231 missing whitespace after ','
Line 142:5: E124 closing bracket does not match visual indentation
Line 158:1: E302 expected 2 blank lines, found 1
Line 159:44: E231 missing whitespace after ','
Line 159:72: E231 missing whitespace after ','
Line 159:83: E231 missing whitespace after ','
Line 159:101: E501 line too long (123 > 100 characters)
Line 159:108: E231 missing whitespace after ','
Line 160:5: E124 closing bracket does not match visual indentation
Line 179:20: E231 missing whitespace after ','
Line 180:27: E231 missing whitespace after ','
Line 180:41: E201 whitespace after '['
Line 181:41: E201 whitespace after '['
Line 182:41: E201 whitespace after '['
Line 184:41: E201 whitespace after '['
Line 195:1: E302 expected 2 blank lines, found 1
Line 196:20: E231 missing whitespace after ','
Line 197:27: E231 missing whitespace after ','
Line 197:41: E201 whitespace after '['
Line 198:41: E201 whitespace after '['
Line 199:41: E201 whitespace after '['
Line 201:41: E201 whitespace after '['
Line 212:1: E302 expected 2 blank lines, found 1
Line 213:20: E231 missing whitespace after ','
Line 214:27: E231 missing whitespace after ','
Line 229:1: E302 expected 2 blank lines, found 1
Line 230:20: E231 missing whitespace after ','
Line 231:27: E231 missing whitespace after ','

Comment last updated at 2020-04-09 16:03:55 UTC

]
)
def test_meta_from_hdulist(header_obj, header_values1, header_values2, header_values3, header_values4, header_values5, missing_axes):
wcs_axes, crpix1, crpix2, crpix3 = header_values1
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I haven't found a way to test meta values, so maybe we need to decide on the way we store meta values? So that we have only one way to access them.

@DanRyanIrish DanRyanIrish added this to the 2.1 milestone Apr 9, 2020

"""

if len(key) > 8 and len(value) > 72:
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary that both these conditions are met? What if len(value) < 72 but len(key) > 8? Is this still correct?

@stale
Copy link

stale bot commented Sep 30, 2020

This pull request has been automatically marked as stale because it has not had any activity for the past five months. It will be closed if no further activity occurs. If the ideas in this pull request are still worth implementing, please make sure you open an issue to keep track of that idea!

@stale stale bot added the stale label Sep 30, 2020
@stale
Copy link

stale bot commented Oct 30, 2020

This pull request has been automatically closed since there was no activity for a month since it was marked as stale. If the ideas in this pull request are still worth implementing, please make sure you open an issue to keep track of that idea!

@stale stale bot closed this Oct 30, 2020
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