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

lazy_import: Muffle diagnostic messages for "at_startup" #32619

Closed
mkoeppe opened this issue Oct 4, 2021 · 22 comments
Closed

lazy_import: Muffle diagnostic messages for "at_startup" #32619

mkoeppe opened this issue Oct 4, 2021 · 22 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 4, 2021

When loading a modularized part of Sage instead of sage.all, we should suppress the second of these two messages:

        if startup_guard and not self._at_startup:
            print(f"Resolving lazy import {self._name} during startup")
        elif self._at_startup and not startup_guard:
            print(f"Option ``at_startup=True`` for lazy import {self._name} not needed anymore")

Also, instead of print, we should use warnings.warn.

Previous discussion:

CC: @tscrim @jhpalmieri @nthiery

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 4a8f409

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/32619

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 4, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

New commits:

7e7f5fesage.doctest.forker: Use sage.misc.lazy_import.ensure_startup_finished

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

Commit: 7e7f5fe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2021

Changed commit from 7e7f5fe to 181e4a0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

181e4a0src/sage/misc/lazy_import.pyx: Use warnings.warn instead of print

@tscrim
Copy link
Collaborator

tscrim commented Oct 4, 2021

comment:5

Green bot => positive review.

@tscrim
Copy link
Collaborator

tscrim commented Oct 4, 2021

Reviewer: Travis Scrimshaw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2021

Changed commit from 181e4a0 to bc2b707

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

bc2b707sage.misc.lazy_import.finish_startup: Fixup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

9830e4esrc/sage/misc/lazy_import.pyx: Adjust doctest output to use of warnings.warn

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2021

Changed commit from bc2b707 to 9830e4e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

dadfa07src/sage/misc/lazy_import.pyx: Fix markup of doctest output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from 9830e4e to dadfa07

@tscrim
Copy link
Collaborator

tscrim commented Oct 6, 2021

comment:11

There still is one doctest failure in misc/lazy_import.pyx.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

98d2e9bsrc/sage/misc/lazy_import.pyx: In test_fake_startup, also reset finish_startup_called

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from dadfa07 to 98d2e9b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 10, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

4a8f409src/sage/misc/lazy_import.pyx: Warnings suppress duplicates, so use a different import for the repeated test

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 10, 2021

Changed commit from 98d2e9b to 4a8f409

@tscrim
Copy link
Collaborator

tscrim commented Oct 10, 2021

comment:16

LGTM.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 10, 2021

comment:17

Thanks!

@vbraun
Copy link
Member

vbraun commented Oct 13, 2021

@vbraun vbraun closed this as completed in e28338b Oct 13, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants