Opened 15 months ago
Closed 13 months ago
#31654 closed enhancement (fixed)
copy_from for scalar fields
Reported by: | gh-mjungmath | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.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 15 months ago by
- Branch set to u/gh-mjungmath/copy_from_scalarfields
comment:2 Changed 15 months ago by
- Commit set to 4d6993e51c7b5f6eb4c92a66d9e68a0652a27425
- Status changed from new to needs_review
- Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 15 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 15 months ago by
Copying the restrictions seems unnecessary. All information should be contained in the expressions.
comment:5 Changed 15 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 15 months ago by
- Branch changed from u/gh-mjungmath/copy_from_scalarfields to public/31654
comment:7 Changed 15 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 15 months ago by
- Branch changed from public/ticket/21991 to u/gh-mjungmath/copy_from_scalarfields
- Commit changed from 13b8f3fa915439e5edbc2df077c22fc679b6593e to 3a2f28572f847e20378e715b2a155f3c00cae0f5
comment:9 Changed 15 months ago by
No worries. I am just happy so see that I am not the only one doing these kind of mistkes. :D
comment:10 Changed 15 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/gh-mjungmath/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 15 months ago by
Branch looks fine.
comment:12 Changed 15 months ago by
Patchbot is green.
comment:13 Changed 15 months ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM.
comment:14 Changed 15 months ago by
Thanks for the review!
comment:15 Changed 15 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 15 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 15 months ago by
Thus, needs a new review (waiting for patchbot).
comment:18 Changed 15 months ago by
green bot => positive review
comment:19 Changed 15 months ago by
- Status changed from needs_review to positive_review
comment:20 Changed 15 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 15 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 15 months ago by
- Status changed from needs_work to needs_review
Done. Thank you. :)
comment:23 Changed 15 months ago by
- Status changed from needs_review to positive_review
comment:24 Changed 15 months ago by
- Milestone changed from sage-9.3 to sage-9.4
comment:25 Changed 13 months ago by
- Branch changed from u/gh-mjungmath/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