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

Rename blowdown to blowup #4148

Merged

Conversation

paemurru
Copy link
Contributor

@paemurru paemurru commented Sep 24, 2024

Command used:

sed --follow-symlinks -i \
-e "s;blowdown;blowup;g" \
-e "s;blow-down;blow-up;g" \
-e "s;Blow-down;Blow-up;g" \
-e "s;Blowdown;Blowup;g" \
-e "s;BlowDown;BlowUp;g" \
$(git ls-files)

mv "./test/AlgebraicGeometry/ToricVarieties/toric_blowdowns.jl" "./test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl"
mv "./docs/src/AlgebraicGeometry/ToricVarieties/BlowdownMorphisms.md" "./docs/src/AlgebraicGeometry/ToricVarieties/BlowupMorphisms.md"

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

Attention: Patch coverage is 62.12121% with 25 lines in your changes missing coverage. Please review.

Project coverage is 84.65%. Comparing base (d91db2a) to head (4ad9d7e).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
experimental/Schemes/src/BlowupMorphism.jl 55.55% 12 Missing ⚠️
experimental/Schemes/src/ToricBlowups/types.jl 57.89% 8 Missing ⚠️
...xperimental/Schemes/src/ToricBlowups/attributes.jl 80.00% 2 Missing ⚠️
experimental/FTheoryTools/src/auxiliary.jl 50.00% 1 Missing ⚠️
...erimental/Schemes/src/ToricBlowups/constructors.jl 80.00% 1 Missing ⚠️
experimental/Schemes/src/ToricBlowups/methods.jl 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4148   +/-   ##
=======================================
  Coverage   84.65%   84.65%           
=======================================
  Files         626      626           
  Lines       84316    84316           
=======================================
  Hits        71380    71380           
  Misses      12936    12936           
Files with missing lines Coverage Δ
experimental/Schemes/src/Resolution_structure.jl 67.53% <100.00%> (ø)
...y/Schemes/CoveredSchemes/Morphisms/Constructors.jl 95.45% <ø> (ø)
...ometry/ToricVarieties/ToricMorphisms/attributes.jl 80.21% <ø> (ø)
experimental/FTheoryTools/src/auxiliary.jl 76.00% <50.00%> (ø)
...erimental/Schemes/src/ToricBlowups/constructors.jl 89.83% <80.00%> (ø)
experimental/Schemes/src/ToricBlowups/methods.jl 50.00% <50.00%> (ø)
...xperimental/Schemes/src/ToricBlowups/attributes.jl 93.54% <80.00%> (ø)
experimental/Schemes/src/ToricBlowups/types.jl 71.92% <57.89%> (ø)
experimental/Schemes/src/BlowupMorphism.jl 89.63% <55.55%> (ø)

... and 1 file with indirect coverage changes

@afkafkafk13
Copy link
Collaborator

Looks good to me. Ping @HereAround for the toric side, please. (In case of his approval, this can be merged.)

@afkafkafk13
Copy link
Collaborator

Do we need to deprecate to save some code that people have written but not merged yet?

@HechtiDerLachs
Copy link
Collaborator

I'm afraid that an n-th layer of deprecation is necessary. Maybe we can even roll back in some cases? I don't know how to handle this. Other than that, I'm ok with the changes.

@lgoettgens
Copy link
Member

lgoettgens commented Sep 25, 2024

From a purely technical POV, deprecations are not necessary here, as there are no (non-comment) changes to src/, and no changes to the book.
However, adding deprecations for things in experimental/ is still a nice convenience for users.

(cc @lkastner)

@mjrodgers
Copy link
Collaborator

Discussed in triage, looks okay to merge.

@afkafkafk13 afkafkafk13 merged commit 08aa6a6 into oscar-system:master Sep 25, 2024
28 checks passed
@paemurru paemurru deleted the ep/rename_blowdown_to_blowup branch September 25, 2024 10:16
HechtiDerLachs pushed a commit to HechtiDerLachs/Oscar.jl that referenced this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants