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

PR from abrombo/galgebra:master for reference (with merge conflicts solved) #62

Closed
wants to merge 20 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Nov 21, 2019

A variant of #52 containing the (trivial) manual merge(s) needed to make the diff readable

abrombo and others added 3 commits October 11, 2019 14:13
function (overloaded print function).  Added function xtex to printer for
displaying LaTeX output.  For example the call xtex(tex='texmaker') invokes
the 'texmaker' program (if installed) with the tex output file.  The is
useful for debugging incorrect LaTeX output.  Replaced sympy collect
function with custom Collect so that tests work with sympy 1.4. Worked
through most of jupyter notebook and LaTeX examples with new python 3
compatible printing in printing.py.  Also added comments to functions
that needed explainations.  Still much more to do on that task.
and Sdop when either is multiplied by (left multiplication) a multivector
resulting in a Dop.

Signed-off-by: Alan Bromborsky <[email protected]>
@eric-wieser
Copy link
Member Author

Created https://github.com/abrombo/galgebra/pull/2 to get this merge into @abrombo's hands

galgebra/mv.py Outdated
Comment on lines 2496 to 1858
def odot(self,dot_flg=True):
new_self = copy.deepcopy(self)
new_self.dot_flg = dot_flg
return new_self
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only change to the differential operators part of this file (xref gh-133)

@eric-wieser eric-wieser force-pushed the abrombo-master-merge branch from 018bb9f to 7bf587e Compare April 22, 2020 14:32
The comment in ga.py is just copied from the sympy docs.
The one in metric is self-explanatory.
This was errantly removed by me in a previous commit, I think
@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2020

This pull request introduces 6 alerts and fixes 1 when merging 3182784 into 409bb90 - view on LGTM.com

new alerts:

  • 5 for Unused local variable
  • 1 for Module imports itself

fixed alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2020

This pull request introduces 6 alerts and fixes 1 when merging 7988e73 into 409bb90 - view on LGTM.com

new alerts:

  • 5 for Unused local variable
  • 1 for Module imports itself

fixed alerts:

  • 1 for Unused local variable

@eric-wieser
Copy link
Member Author

Rebased with some flake8 fixes and removals of stray files.

I think all that's really in here now is an incomplete odot implementation, and the printer stuff.

@lgtm-com
Copy link

lgtm-com bot commented Apr 23, 2020

This pull request introduces 6 alerts and fixes 1 when merging bfeaddb into 46cde16 - view on LGTM.com

new alerts:

  • 5 for Unused local variable
  • 1 for Module imports itself

fixed alerts:

  • 1 for Unused local variable

galgebra/mv.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 25, 2020

This pull request introduces 6 alerts and fixes 1 when merging 9f8a1ec into ce2daf8 - view on LGTM.com

new alerts:

  • 5 for Unused local variable
  • 1 for Module imports itself

fixed alerts:

  • 1 for Unused local variable

Comment on lines +1118 to +1121
from IPython.core.interactiveshell import InteractiveShell
# Allow multiple outputs in an output cell
# https://forums.fast.ai/t/jupyter-notebook-enhancements-tips-and-tricks/17064/2
InteractiveShell.ast_node_interactivity = "all"
Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't about allowing multiple outputs, it's about producing them automatically.
The user can always print multiple outputs with display(...) if they want to.

This doesn't seem like a good idea to me. If users want it, they can do it themselves.

@lgtm-com
Copy link

lgtm-com bot commented Apr 29, 2020

This pull request introduces 6 alerts and fixes 1 when merging 1234445 into 421449b - view on LGTM.com

new alerts:

  • 5 for Unused local variable
  • 1 for Module imports itself

fixed alerts:

  • 1 for Unused local variable

Comment on lines +1040 to +1044
if isinstance(s[0], str):

if s[0] == 'h':
GaLatexPrinter.fmt_dict['h'] = True
s = s[1:]
Copy link
Member Author

Choose a reason for hiding this comment

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

This looks like a bad idea to me. This changes the behavior of print(my_str) to print nothing at all if my_str == 'h'!

@eric-wieser eric-wieser added the component: printer string, latex, and ansi escape printing label Apr 30, 2020
@eric-wieser
Copy link
Member Author

Conflicts solved again

@lgtm-com
Copy link

lgtm-com bot commented May 15, 2020

This pull request introduces 6 alerts when merging 448edf0 into 5108d16 - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 1 for Module imports itself
  • 1 for Unused import

@eric-wieser eric-wieser force-pushed the abrombo-master-merge branch from 448edf0 to 9aa5005 Compare May 15, 2020 11:03
@codecov
Copy link

codecov bot commented May 15, 2020

Codecov Report

Merging #62 into master will decrease coverage by 1.70%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
- Coverage   77.56%   75.86%   -1.71%     
==========================================
  Files          16       16              
  Lines        4217     4495     +278     
==========================================
+ Hits         3271     3410     +139     
- Misses        946     1085     +139     
Impacted Files Coverage Δ
galgebra/printer.py 71.73% <0.00%> (-6.90%) ⬇️
galgebra/dop.py 88.52% <0.00%> (-2.55%) ⬇️
galgebra/lt.py 57.61% <0.00%> (-1.69%) ⬇️
galgebra/atoms.py 97.33% <0.00%> (-0.35%) ⬇️
galgebra/ga.py 80.64% <0.00%> (-0.13%) ⬇️
galgebra/metric.py 76.04% <0.00%> (-0.12%) ⬇️
galgebra/deprecated.py 100.00% <0.00%> (ø)
galgebra/mv.py 74.91% <0.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f57a09...76315e4. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented May 15, 2020

This pull request introduces 6 alerts when merging 9aa5005 into 5108d16 - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 1 for Module imports itself
  • 1 for Unused import

Comment on lines 51 to +54
\newcommand{\lbrc}{\left \{}
\newcommand{\rbrc}{\right \}}
\newcommand{\rbrc}{\right \]}
\newcommand{\lbrk}{\left \[}
\newcommand{\rbrk}{\right \}}
Copy link
Member Author

Choose a reason for hiding this comment

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

@brombo, this looks to me like a bug?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, obviously

@lgtm-com
Copy link

lgtm-com bot commented May 17, 2020

This pull request introduces 5 alerts when merging 309bbd7 into f6452ab - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 1 for Unused import

…master-merge

# Conflicts:
#	examples/ipython/dop.ipynb
#	examples/ipython/gr_metrics.ipynb
#	galgebra/printer.py
…master-merge

# Conflicts:
#	examples/LaTeX/Dop.py
#	examples/LaTeX/christoffel_symbols.py
#	examples/LaTeX/colored_christoffel_symbols.py
#	examples/ipython/colored_christoffel_symbols.ipynb
#	examples/ipython/inner_product.ipynb
#	examples/ipython/second_derivative.ipynb
#	examples/ipython/st4.ipynb
#	galgebra/printer.py
# Conflicts:
#	examples/ipython/tutorial_algebra.ipynb
#	galgebra/mv.py
#	galgebra/printer.py
@lgtm-com
Copy link

lgtm-com bot commented May 26, 2020

This pull request introduces 5 alerts when merging 76315e4 into 09d882f - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 1 for Unused import

@utensil utensil added this to the 0.6.0 milestone May 15, 2024
@utensil
Copy link
Member

utensil commented May 15, 2024

Thank you @abrombo @eric-wieser , this should be superseded by #510 which merged gprinter from upstream. Feel free to reopen if there are remaining issues.

@utensil utensil closed this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: printer string, latex, and ansi escape printing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants