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

Update tensors.py #935

Merged
merged 7 commits into from
Nov 16, 2023
Merged

Update tensors.py #935

merged 7 commits into from
Nov 16, 2023

Conversation

Li-Xiang-Ideal
Copy link
Contributor

Add LeviCivitaTensor

Add LeviCivitaTensor
@rocky
Copy link
Member

rocky commented Nov 16, 2023

Pretty good adherence to myriad of little details we have in adding a new built-in function. Looks like you should run isort over this. If you install pre-commit then these checks will be run at git commit time. And the funny symbols that are allowed in side a docstring have to be reduced until sometime in the future when we ditch this home-grown documentation system.

I suppose mentioning these things too is missing from the developer's guide.

A couple of other small things. Please move those Python import statements from inside functions to the top. Yeah, I find this counter-intuitive. In fact much of the Python style stuff I find counter-intuitive or wrong. But I have been beaten down by so many Python fascists who insist there is "only one way to write things in Python" that I have given up here.

Also, please consider adding an entry to CHANGES.rst.

But again, all of this is very small stuff. Thanks for doing this.

CHANGES.rst Outdated Show resolved Hide resolved
never used isort or black before :(
Hope it passes the checks this time.
@Li-Xiang-Ideal
Copy link
Contributor Author

First time to experience isort and black 😅 Hope it passes the checks this time.

@mmatera
Copy link
Contributor

mmatera commented Nov 16, 2023

Very nice! Thank you very much! LGTM

@rocky
Copy link
Member

rocky commented Nov 16, 2023

Very nice! Thank you very much! LGTM

I agree! Thanks for sticking with this.

@rocky rocky merged commit e7b7cca into Mathics3:master Nov 16, 2023
11 checks passed
@Li-Xiang-Ideal Li-Xiang-Ideal deleted the patch-1 branch November 16, 2023 13:11
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.

3 participants