Skip to content

Allow initialization of zero-size Times for all formats#8854

Merged
taldcroft merged 3 commits intoastropy:masterfrom
mhvk:time-zero-size-ok
Jul 18, 2019
Merged

Allow initialization of zero-size Times for all formats#8854
taldcroft merged 3 commits intoastropy:masterfrom
mhvk:time-zero-size-ok

Conversation

@mhvk
Copy link
Copy Markdown
Contributor

@mhvk mhvk commented Jun 15, 2019

Seeing astro-bot warn about #7868 again, I thought I would just take it over and make a PR that uses a bit less special-casing for zero-size (though still more than I'd like, but hard to avoid).

Milestoned this for 3.2.1 as I think it may be hard to backport.

Fixes #7866

@mhvk mhvk requested a review from taldcroft June 15, 2019 17:05
@mhvk mhvk force-pushed the time-zero-size-ok branch from c6293e2 to 95c7818 Compare June 15, 2019 17:06
@mhvk mhvk added this to the v3.2.2 milestone Jun 15, 2019
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 15, 2019

Codecov Report

Merging #8854 into master will increase coverage by 0.41%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8854      +/-   ##
==========================================
+ Coverage   86.57%   86.99%   +0.41%     
==========================================
  Files         405      400       -5     
  Lines       60098    59482     -616     
  Branches     1100     1100              
==========================================
- Hits        52032    51747     -285     
+ Misses       7425     7094     -331     
  Partials      641      641
Impacted Files Coverage Δ
astropy/time/formats.py 97.53% <100%> (ø) ⬆️
astropy/visualization/wcsaxes/coordinates_map.py 86.88% <0%> (-10.62%) ⬇️
astropy/constants/__init__.py 94.87% <0%> (-5.13%) ⬇️
astropy/io/misc/pickle_helpers.py 97.77% <0%> (-2.23%) ⬇️
astropy/visualization/wcsaxes/utils.py 99.01% <0%> (-0.99%) ⬇️
astropy/visualization/wcsaxes/transforms.py 90% <0%> (-0.91%) ⬇️
astropy/io/fits/convenience.py 85.76% <0%> (-0.8%) ⬇️
astropy/utils/iers/iers.py 92.16% <0%> (-0.78%) ⬇️
astropy/stats/bayesian_blocks.py 95.3% <0%> (-0.68%) ⬇️
astropy/coordinates/earth.py 75.84% <0%> (-0.38%) ⬇️
... and 86 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfbb7e5...af6d8cf. Read the comment docs.

@astropy astropy deleted a comment from codecov bot Jul 5, 2019
@mhvk mhvk mentioned this pull request Jul 15, 2019
Copy link
Copy Markdown
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

Looks good, just an informational question, and needs a rebase.

flags=['refs_ok'],
op_dtypes=[object] + 5*[np.intc] + [np.double])
flags=['refs_ok', 'zerosize_ok'],
op_dtypes=[None] + 5*[np.intc] + [np.double])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's going on with the change in op_dtypes everywhere w/r/t to the val type to None? I could not find any docs that suggest what should happen in this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

for inputs, None means to just take the dtype from the input. This is needed since for empty arrays, the dtype will not be object (and there is little point correcting it since no iteration will happen anyway)

@mhvk mhvk force-pushed the time-zero-size-ok branch from 95c7818 to 476a447 Compare July 16, 2019 13:51
@mhvk mhvk force-pushed the time-zero-size-ok branch from 476a447 to af6d8cf Compare July 16, 2019 13:54
@mhvk
Copy link
Copy Markdown
Contributor Author

mhvk commented Jul 16, 2019

OK, rebased

Copy link
Copy Markdown
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

👍 thanks!

@taldcroft taldcroft merged commit fba0e50 into astropy:master Jul 18, 2019
@mhvk mhvk deleted the time-zero-size-ok branch July 18, 2019 14:26
bsipocz pushed a commit that referenced this pull request Jul 22, 2019
Allow initialization of zero-size Times for all formats
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow empty Time objects

2 participants