sage.{algebras,combinat,matroids}: Replace imports from sage.*.all for namespace packages #35090

mkoeppe · 2023-02-12T14:00:39Z

📚 Description

Fixes #34946

📝 Checklist

I have made sure that the title is self-explanatory and the description concisely explains the PR.
I have linked an issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

…Q/;s/ Z as ZZ/ ZZ/;'

…ce_imports_from_sage___all_for_namespace_packages SageMath version 9.8, Release Date: 2023-02-11

codecov-commenter · 2023-02-12T17:04:31Z

Codecov Report

Base: 88.60% // Head: 88.58% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (912e4c5) compared to base (8f5bbd2).
Patch coverage: 98.59% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #35090      +/-   ##
===========================================
- Coverage    88.60%   88.58%   -0.02%     
===========================================
  Files         2140     2140              
  Lines       397273   397287      +14     
===========================================
- Hits        352004   351950      -54     
- Misses       45269    45337      +68

Impacted Files	Coverage Δ
...age/algebras/hecke_algebras/ariki_koike_algebra.py	`96.03% <0.00%> (ø)`
src/sage/combinat/words/morphism.py	`92.51% <0.00%> (ø)`
...e/combinat/root_system/reflection_group_complex.py	`23.03% <66.66%> (+0.14%)`	⬆️
src/sage/algebras/affine_nil_temperley_lieb.py	`98.55% <100.00%> (ø)`
src/sage/algebras/cluster_algebra.py	`96.69% <100.00%> (ø)`
src/sage/algebras/free_algebra.py	`96.16% <100.00%> (ø)`
src/sage/algebras/free_algebra_quotient.py	`98.63% <100.00%> (ø)`
src/sage/algebras/group_algebra.py	`94.11% <100.00%> (ø)`
src/sage/algebras/iwahori_hecke_algebra.py	`82.76% <100.00%> (+0.07%)`	⬆️
src/sage/algebras/lie_algebras/bch.py	`100.00% <100.00%> (ø)`
... and 109 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

…__replace_imports_from_sage___all_for_namespace_packages SageMath version 10.0.beta0, Release Date: 2023-02-12

…__replace_imports_from_sage___all_for_namespace_packages

tscrim

Two basically trivially things if you want to do them:

The imports from sage.arith.misc import GCD as gcd are somewhat unnecessary (like the Q as QQ).
The FiniteField as GF in the matroids, similarly. I am wondering if we should just replace the GF with FiniteField.

…gebras_combinat_matroids___replace_imports_from_sage___all_for_namespace_packages

…s sed -i.bak 's/GCD as gcd/gcd/'

mkoeppe · 2023-02-14T19:33:05Z

The imports from sage.arith.misc import GCD as gcd are somewhat unnecessary (like the Q as QQ).

Thanks, indeed gcd is also provided by sage.arith.misc; so I've made this simplification.

The FiniteField as GF in the matroids, similarly. I am wondering if we should just replace the GF with FiniteField.

I'll not do this one here; importing it as GF seems like a convenient abbreviation

tscrim

Fair enough. LGTM.

mkoeppe · 2023-02-16T07:30:13Z

Thank you!

src/sage/combinat/species/cycle_species.py

tobiasdiez · 2023-02-18T14:37:00Z

src/sage/matroids/linear_matroid.pyx

+from sage.rings.finite_rings.finite_field_constructor import FiniteField
+from sage.rings.finite_rings.finite_field_constructor import FiniteField as GF


These imports seem to be duplicates

Yes, and the first one was not even used. I've cleaned it up now

…orts

…gebras_combinat_matroids___replace_imports_from_sage___all_for_namespace_packages

github-actions · 2023-02-24T03:04:09Z

Documentation preview for this PR is ready! 🎉
Built with commit: 912e4c5

…ages  ### 📚 Description Fixes #34201    ### 📝 Checklist    - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   - Depends on #35090 - Depends on #35098 - Depends on #35099 - Depends on #35105 - Depends on #35106 - Depends on #35107 URL: #35110 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik

alexchandler100 and others added 4 commits January 29, 2023 10:46

sage -fiximports src/sage/algebras src/sage/combinat src/sage/matroids

bdf2f33

git grep -l -E ' (Q as QQ|Z as ZZ)' | xargs sed -i.bak 's/ Q as QQ/ Q…

bd50b49

…Q/;s/ Z as ZZ/ ZZ/;'

sage.{algebras,combinat}: Consolidate imports from the same module

8e77597

Merge tag '9.8' into t/34946/sage__algebras_combinat_matroids___repla…

f8bf304

…ce_imports_from_sage___all_for_namespace_packages SageMath version 9.8, Release Date: 2023-02-11

mkoeppe added s: positive review t: refactoring labels Feb 12, 2023

Matthias Koeppe added 2 commits February 12, 2023 16:39

Merge tag '10.0.beta0' into t/34946/sage__algebras_combinat_matroids_…

7bb289c

…__replace_imports_from_sage___all_for_namespace_packages SageMath version 10.0.beta0, Release Date: 2023-02-12

Merge branch 'develop' into t/34946/sage__algebras_combinat_matroids_…

254431c

…__replace_imports_from_sage___all_for_namespace_packages

tscrim requested changes Feb 14, 2023

View reviewed changes

mkoeppe added s: needs work and removed s: positive review labels Feb 14, 2023

Matthias Koeppe added 3 commits February 14, 2023 11:21

Merge remote-tracking branch 'upstream/develop' into t/34946/sage__al…

3be20ec

…gebras_combinat_matroids___replace_imports_from_sage___all_for_namespace_packages

src/sage/combinat/necklace.py: Add missing import

ed1ee83

git grep -l 'GCD as gcd' src/sage/{algebras,combinat,matroids} | xarg…

10d2e1c

…s sed -i.bak 's/GCD as gcd/gcd/'

mkoeppe added s: needs review and removed s: needs work labels Feb 14, 2023

mkoeppe requested a review from tscrim February 16, 2023 05:55

tscrim approved these changes Feb 16, 2023

View reviewed changes

mkoeppe added s: positive review and removed s: needs review labels Feb 16, 2023

tobiasdiez suggested changes Feb 16, 2023

View reviewed changes

src/sage/combinat/species/cycle_species.py Outdated Show resolved Hide resolved

tobiasdiez added s: needs work and removed s: positive review labels Feb 16, 2023

Replace relative imports by absolute imports because pytest

29ad54e

mkoeppe added s: needs review and removed s: needs work labels Feb 16, 2023

mkoeppe requested a review from tobiasdiez February 16, 2023 22:38

mkoeppe self-assigned this Feb 18, 2023

tobiasdiez reviewed Feb 18, 2023

View reviewed changes

src/sage/matroids/linear_matroid.pyx: Remove unused imports, sort imp…

8c2d315

…orts

mkoeppe requested a review from tobiasdiez February 18, 2023 19:09

tobiasdiez approved these changes Feb 19, 2023

View reviewed changes

tobiasdiez added s: positive review and removed s: needs review labels Feb 19, 2023

Merge remote-tracking branch 'upstream/develop' into t/34946/sage__al…

912e4c5

…gebras_combinat_matroids___replace_imports_from_sage___all_for_namespace_packages

vbraun merged commit b0df140 into sagemath:develop Mar 19, 2023

mkoeppe removed the s: positive review label Mar 19, 2023

mkoeppe added this to the sage-10.0 milestone Mar 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sage.{algebras,combinat,matroids}: Replace imports from sage.*.all for namespace packages #35090

sage.{algebras,combinat,matroids}: Replace imports from sage.*.all for namespace packages #35090

mkoeppe commented Feb 12, 2023

codecov-commenter commented Feb 12, 2023 •

edited

Loading

tscrim left a comment

mkoeppe commented Feb 14, 2023

tscrim left a comment

mkoeppe commented Feb 16, 2023

tobiasdiez Feb 18, 2023

mkoeppe Feb 18, 2023

github-actions bot commented Feb 24, 2023

		from sage.rings.finite_rings.finite_field_constructor import FiniteField
		from sage.rings.finite_rings.finite_field_constructor import FiniteField as GF

sage.{algebras,combinat,matroids}: Replace imports from sage.*.all for namespace packages #35090

sage.{algebras,combinat,matroids}: Replace imports from sage.*.all for namespace packages #35090

Conversation

mkoeppe commented Feb 12, 2023

📚 Description

📝 Checklist

⌛ Dependencies

codecov-commenter commented Feb 12, 2023 • edited Loading

Codecov Report

tscrim left a comment

Choose a reason for hiding this comment

mkoeppe commented Feb 14, 2023

tscrim left a comment

Choose a reason for hiding this comment

mkoeppe commented Feb 16, 2023

tobiasdiez Feb 18, 2023

Choose a reason for hiding this comment

mkoeppe Feb 18, 2023

Choose a reason for hiding this comment

github-actions bot commented Feb 24, 2023

codecov-commenter commented Feb 12, 2023 •

edited

Loading