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 ctranspose to adjoint #23235

Merged
merged 5 commits into from
Aug 22, 2017
Merged

Rename ctranspose to adjoint #23235

merged 5 commits into from
Aug 22, 2017

Conversation

andyferris
Copy link
Member

Just to prove exactly how seriously we take transposes, we need to rename one of them.

Part of #20978. This one was simple find-replace (so far).

NEWS.md Outdated
@@ -309,6 +309,9 @@ Deprecated or removed
* `Base.cpad` has been removed; use an appropriate combination of `rpad` and `lpad`
instead ([#23187]).

* `ctranspose` and `ctranspose!` have been deprecated in favor of `adjoint` and `adjoint!`,
respectively.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the pull request number? :)

@@ -1684,6 +1684,10 @@ end
# PR #22742: change in isapprox semantics
@deprecate rtoldefault(x,y) rtoldefault(x,y,0) false

# PR #23235
@deprecate ctranspose adjoint
@deprecate ctranspose! adjoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing !?

@@ -49,7 +49,7 @@ p = 1=>:foo
@test xor(2) == 2
@test (⊻)(2) == 2

# @test ctranspose('a') == 'a' # (c)transpose of Chars no longer supported
# @test adjoint('a') == 'a' # (c)transpose of Chars no longer supported
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps update the comment as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I think we can delete this now.

(I have an upcoming PR which will let us transpose arrays of Chars and Strings and so-on, so the context of this whole line will be rather out-of-date.)

@Sacha0 Sacha0 added deprecation This change introduces or involves a deprecation linear algebra Linear algebra labels Aug 13, 2017
@Sacha0 Sacha0 added this to the 1.0 milestone Aug 13, 2017
@Sacha0 Sacha0 added the maths Mathematical functions label Aug 13, 2017
@andyferris
Copy link
Member Author

Any more comments or any detractors before I merge this?

(I just realized we should probably rename A_mul_Bc to A_mul_Ba etc, but I won't do that because we plan to remove these entirely in this release cycle.)

@StefanKarpinski
Copy link
Member

Let's give it a bit more time since this has only been open for half a day on a weekend. I don't imagine there being much controversy here, however.

@andyferris
Copy link
Member Author

Something went funky with the linux tests

@StefanKarpinski
Copy link
Member

Restarted them.

@StefanKarpinski
Copy link
Member

Bump – rebase. That Linux issue should be resolved now.

@ViralBShah
Copy link
Member

cc @alanedelman

@andyferris
Copy link
Member Author

Thanks @andreasnoack (I'm travelling ATM).

The test errors appear unrelated and oddly spurious... one inference failure for RowVector and test failure one in codegen, but only on specific builds.

@andreasnoack andreasnoack merged commit df9b1b0 into master Aug 22, 2017
@andreasnoack andreasnoack deleted the ajf/adjoint branch August 22, 2017 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation linear algebra Linear algebra maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants