Skip to content

Knot theory fixes - #1199

Closed
mmatera wants to merge 17 commits into
masterfrom
KnotTheoryFixes
Closed

Knot theory fixes#1199
mmatera wants to merge 17 commits into
masterfrom
KnotTheoryFixes

Conversation

@mmatera

Copy link
Copy Markdown
Contributor

No description provided.

@rocky

Copy link
Copy Markdown
Member

@mmatera think I understand how to write the type table-lookup code I was describing in a rough way.

What would help though is are some useful tests that come up.

Comment threadmathics/builtin/files.py Outdated

# Wrap types in a list (if it isn't already one)
if not types.has_form("List", None):
if types.get_head_name() == "System`Hold":

@rockyrockyMar 25, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems hoaky since the specification of Read[] seems to allow any function and not just Hold[] right?

@rockyrocky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please let's start splitting this up into the individual pieces and create separate PRs for this.

For example Setting System`Notebooks is a thing in of itself and not even needed for Feyncalc.

That could be merged in by itself.

The problem with these growing grab-bag PRs is that they are impossible to review and the quality can suffer because we can't focus on individual features.

And then they are continually merged (when rebasing instead of merging would lead to more comprehensible results)

Comment threadmathics/builtin/strings.py Outdated
<dd>splits $s$ at the delimiter $d$.
<dt>'StringSplit[$s$, {"$d1$", "$d2$", ...}]'
<dd>splits $s$ using multiple delimiters.
<dt>'StringSplit[{$s_1$, $s_2, $\ldots$}, {"$d1$", "$d2$", ...}]'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note that ldots doesn't render in the Django interface.

fix duplicated DirectedInfinity entry
restoring tests
Fix Positive/Negative tests
clean and working
comment out debuggin prints
clean and improved tests
fix duplicated DirectedInfinity entry
clean and improved tests
fix duplicated DirectedInfinity entry
restoring tests
Fix Positive/Negative tests
clean and working
reverting changes in modules
splitting working and not working tests...
split in working, possible working with small changes, and not working
@mmaterammatera mentioned this pull request Mar 31, 2021
@mmatera

Copy link
Copy Markdown
ContributorAuthor

Spli

Please let's start splitting this up into the individual pieces and create separate PRs for this.

For example Setting System`Notebooks is a thing in of itself and not even needed for Feyncalc.

That could be merged in by itself.

The problem with these growing grab-bag PRs is that they are impossible to review and the quality can suffer because we can't focus on individual features.

And then they are continually merged (when rebasing instead of merging would lead to more comprehensible results)

Splitted on #1219, #1209#1216, #1217

@mmatera
mmatera requested a review from rockyApril 1, 2021 21:51
@rockyrocky mentioned this pull request Apr 3, 2021
@rocky

rocky commented Apr 3, 2021

Copy link
Copy Markdown
Member

Superceded by other PRs.

@rockyrocky closed this Apr 3, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mmatera@rocky