Skip to content

Conversation

@pllim
Copy link
Member

@pllim pllim commented Feb 24, 2023

throwing exception
@github-actions
Copy link
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@github-actions
Copy link
Contributor

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@pllim
Copy link
Member Author

pllim commented Feb 24, 2023

Is it related to this? 🤷‍♀️

@pllim
Copy link
Member Author

pllim commented Feb 24, 2023

Hmm could the failure only happen in Python 3.11.2 + numpy-dev combo?

@pllim
Copy link
Member Author

pllim commented Feb 24, 2023

I cannot reproduce this with Python 3.11.0 from conda and numpy nightly wheel. The log in CI says it is using Python 3.11.2.

@pllim
Copy link
Member Author

pllim commented Feb 24, 2023

Last green devdeps: https://github.com/astropy/astropy/actions/runs/4247771490/jobs/7386198567 (before all the asdf deprecations getting in the way)

Full Python Version: 
3.11.2 (main, Feb  8 2023, 08:38:01) [GCC 11.3.0]

Numpy: 1.25.0.dev0+678.g51ecf8401

Now with this failure: https://github.com/astropy/astropy/actions/runs/4265444197/jobs/7424786551

Full Python Version: 
3.11.2 (main, Feb  8 2023, 08:38:01) [GCC 11.3.0]

Numpy: 1.25.0.dev0+787.g3f142646c

@pllim
Copy link
Member Author

pllim commented Feb 24, 2023

@mhvk
Copy link
Contributor

mhvk commented Feb 25, 2023

Almost certainly numpy/numpy#22924 given the "RuntimeError: Internal error, tried to fetch clear function for the user dtype '�' without fields or subarray (legacy support)", which was introduced there...

@mhvk
Copy link
Contributor

mhvk commented Feb 25, 2023

Our failure looks due to astropy/io/votable/tests/table_test.py, specifically test_binary2_masked_strings, but it does not raise if one just does the one test -- it requires test_no_field_not_empty_table to be run beforehand to cause the problem (all others can be commented out and do not change the result).

Indeed, the latter seems to be the real problem. in a separate ipython session, I get

In [1]: from astropy.io.votable.tests.table_test import test_no_field_not_empty_table

In [2]: test_no_field_not_empty_table()

In [3]:                                                                                                         
Do you really want to exit ([y]/n)? 
Exception ignored in tp_clear of: <class 'astropy.io.votable.tree.VOTableFile'>
Traceback (most recent call last):
  File "/data/mhvk/venv/numpy-dev2/lib/python3.11/site-packages/IPython/core/displayhook.py", line 317, in flush
    gc.collect()
RuntimeError: Internal error, tried to fetch clear function for the user dtype '' without fields or subarray (legacy support).

So, it clearly is something that happens only at clean-up time.

@mhvk
Copy link
Contributor

mhvk commented Feb 25, 2023

Minimal example to reproduce:

In [1]: from astropy.utils.data import get_pkg_data_filename

In [2]: from astropy.io.votable.table import parse

In [3]: votable = parse(get_pkg_data_filename("data/no_field_not_empty_table.xml", package='astropy.io.votable.t
   ...: ests'))

In [4]:                                                                                                         
Do you really want to exit ([y]/n)? 
Exception ignored in tp_clear of: <class 'astropy.io.votable.tree.VOTableFile'>
RuntimeError: Internal error, tried to fetch clear function for the user dtype '' without fields or subarray (legacy support).

@mhvk
Copy link
Contributor

mhvk commented Feb 25, 2023

Digging deeper, I think this may be the dtype causing a problem

In [26]: votable.resources[0].tables[0].array
Out[26]: 
masked_recarray(data=[],
                mask=[],
          fill_value=b'???',
               dtype='|V8')

This leads to a pure numpy example - see numpy/numpy#23277

@pllim
Copy link
Member Author

pllim commented Feb 27, 2023

@pllim pllim closed this Feb 27, 2023
@pllim pllim deleted the whats-going-on branch February 27, 2023 19:50
@mhvk
Copy link
Contributor

mhvk commented Feb 27, 2023

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants