Skip to content

Conversation

@mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Mar 22, 2024

In fact, we remove the use of SAGE_LIB and replace what depended on this by recognizing certain package data directories of the Sage library.

Fixes doctest failures reported in #37645.

Fixes #22445.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 22, 2024

@tornaria Please take a look if this is solving the problem for you. (I haven't tested very thoroughly yet.)

@kiwifb
Copy link
Member

kiwifb commented Mar 24, 2024

I will have to look at this carefully.

@tornaria
Copy link
Contributor

@tornaria Please take a look if this is solving the problem for you. (I haven't tested very thoroughly yet.)

No, it doesn't. As a matter of fact in my example from #37645 sage.doctest.sources.get_basename gives the correct output already without this PR. It's only when running the test that it prints just the name of the package without qualification (i.e. tolerance instead of sage...).

Thus, this PR is not fixing anything broken.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2024

As a matter of fact in my example from #37645 sage.doctest.sources.get_basename gives the correct output already without this PR. It's only when running the test that it prints just the name of the package without qualification (i.e. tolerance instead of sage...).

That's rather surprising. If you have a moment, could you share how you tested this?

@tornaria
Copy link
Contributor

As a matter of fact in my example from #37645 sage.doctest.sources.get_basename gives the correct output already without this PR. It's only when running the test that it prints just the name of the package without qualification (i.e. tolerance instead of sage...).

That's rather surprising. If you have a moment, could you share how you tested this?

The link is up there in the description of #37645: #37138 (comment)

I did exactly what I describe in that comment (which passes all tests in normal mode) and then repeat the last line but with --long (the failing tests in sage.doctest.test are all # long time).

@github-actions
Copy link

github-actions bot commented Apr 1, 2024

Documentation preview for this PR (built with commit 2f81d78; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Matthias Koeppe and others added 6 commits June 22, 2024 16:15
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.

doctest: Make sage.doctest.sources.get_basename work in more situations

4 participants