Opened 6 months ago
Closed 4 months ago
#31654 closed enhancement (fixed)
copy_from for scalar fields
Reported by:  ghmjungmath  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  manifolds  Keywords:  scalarfields 
Cc:  egourgoulhon, tscrim  Merged in:  
Authors:  Michael Jung  Reviewers:  Travis Scrimshaw, Samuel Lelièvre 
Report Upstream:  N/A  Work issues:  
Branch:  e15fbf3 (Commits, GitHub, GitLab)  Commit:  e15fbf3743e34666aff1b97dfc14d407e269688c 
Dependencies:  Stopgaps: 
Description
In this ticket we equip scalar fields with a method copy_from
as we did for tensor fields in #28628.
Change History (25)
comment:1 Changed 6 months ago by
 Branch set to u/ghmjungmath/copy_from_scalarfields
comment:2 Changed 6 months ago by
 Commit set to 4d6993e51c7b5f6eb4c92a66d9e68a0652a27425
 Status changed from new to needs_review
 Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 6 months ago by
 Commit changed from 4d6993e51c7b5f6eb4c92a66d9e68a0652a27425 to 075f512c8c10f66d04a7d1d5110af20984bc047b
Branch pushed to git repo; I updated commit sha1. New commits:
075f512  Trac #31654: copy of restrictions unnecessary

comment:4 Changed 6 months ago by
Copying the restrictions seems unnecessary. All information should be contained in the expressions.
comment:5 Changed 6 months ago by
 Commit changed from 075f512c8c10f66d04a7d1d5110af20984bc047b to 3a2f28572f847e20378e715b2a155f3c00cae0f5
Branch pushed to git repo; I updated commit sha1. New commits:
3a2f285  Trac #31654: minor typo

comment:6 Changed 6 months ago by
 Branch changed from u/ghmjungmath/copy_from_scalarfields to public/31654
comment:7 Changed 6 months ago by
 Branch changed from public/31654 to public/ticket/21991
 Commit changed from 3a2f28572f847e20378e715b2a155f3c00cae0f5 to 13b8f3fa915439e5edbc2df077c22fc679b6593e
Sorry!!! I pushed my branch to the wrong ticket (typo).
New commits:
13b8f3f  #21991 regression test for issue fixed in #24371

comment:8 Changed 6 months ago by
 Branch changed from public/ticket/21991 to u/ghmjungmath/copy_from_scalarfields
 Commit changed from 13b8f3fa915439e5edbc2df077c22fc679b6593e to 3a2f28572f847e20378e715b2a155f3c00cae0f5
comment:9 Changed 6 months ago by
No worries. I am just happy so see that I am not the only one doing these kind of mistakes. :D
comment:10 Changed 6 months ago by
Thanks very much for your understanding. Please be sure your ticket is set back to the correct branch. I tried to fix it, but I don't really understand trac, so I'm confused and I think may have made things worse: I set the branch to u/ghmjungmath/copy_from_scalarfields
but it looks like maybe it should be public/ticket/21991
. (PS my branch was intended for #31645, and is now there.)
comment:11 Changed 6 months ago by
Branch looks fine.
comment:12 Changed 6 months ago by
Patchbot is green.
comment:13 Changed 6 months ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:14 Changed 6 months ago by
Thanks for the review!
comment:15 Changed 6 months ago by
 Commit changed from 3a2f28572f847e20378e715b2a155f3c00cae0f5 to 3ea7920db5451a1d28d3f604a275c36602d2c1b3
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:
3ea7920  Trac #31654: add copy_from to scalarfields

comment:16 Changed 6 months ago by
Sorry, I was a little sloppy here and simply copied the code. This name thingy is actually not needed.
comment:17 Changed 6 months ago by
Thus, needs a new review (waiting for patchbot).
comment:18 Changed 6 months ago by
green bot => positive review
comment:19 Changed 6 months ago by
 Status changed from needs_review to positive_review
comment:20 Changed 6 months ago by
 Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Samuel Lelièvre
 Status changed from positive_review to needs_work
In both tensorfield.py
and scalarfield.py
, maybe change
 Make ``self`` to a copy of ``other``. + Make ``self`` a copy of ``other``.
Maybe rephrase this:
  ``other``  other scalar field in the very same module from which  ``self`` should be a copy of +  ``other``  other scalar field, in the same module as ``self``
Maybe remove the + sign here?
raise TypeError("the original must be an element "  + "of {}".format(self.parent())) + "of {}".format(self.parent()))
Or feel free to ignore these suggestions and move back to positive review.
comment:21 Changed 6 months ago by
 Commit changed from 3ea7920db5451a1d28d3f604a275c36602d2c1b3 to e15fbf3743e34666aff1b97dfc14d407e269688c
Branch pushed to git repo; I updated commit sha1. New commits:
e15fbf3  Trac #31654: docstring improvements

comment:22 Changed 6 months ago by
 Status changed from needs_work to needs_review
Done. Thank you. :)
comment:23 Changed 6 months ago by
 Status changed from needs_review to positive_review
comment:24 Changed 6 months ago by
 Milestone changed from sage9.3 to sage9.4
comment:25 Changed 4 months ago by
 Branch changed from u/ghmjungmath/copy_from_scalarfields to e15fbf3743e34666aff1b97dfc14d407e269688c
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Trac #31654: add copy_from to scalarfields