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

Use mathics scanner release #1146

Merged
merged 2 commits into from
Feb 6, 2021
Merged

Use mathics scanner release #1146

merged 2 commits into from
Feb 6, 2021

Conversation

rocky
Copy link
Member

@rocky rocky commented Feb 6, 2021

The PossibleZeroQ failure I think was a break from before and another branch may fix.

The RecursionLimit failure I'll investigate later.

@rocky rocky requested review from mmatera and GarkGarcia February 6, 2021 17:32
@rocky rocky force-pushed the use-mathics-scanner-release branch from 4792f48 to 3aac712 Compare February 6, 2021 17:34
Customize LineFeeder Routines for Mathics.
@mmatera
Copy link
Contributor

mmatera commented Feb 6, 2021

@rocky
Copy link
Member Author

rocky commented Feb 6, 2021

Would you please pull out the PossibleZeroQ and put that in its own PR. We'll merge that soon then

  • I restore SingleLine.send_messages in method in mathics-scanner.feeder, get the CI passing again.

This PR addresses that. So that change can be reverted. Thanks.

I am merging this in now. Feel free to keep commenting on it or improve it though.

Merging in will allow mathicsscript and mathics-django to move closer to a release.

@rocky rocky merged commit 6f3ce3a into master Feb 6, 2021
@mmatera
Copy link
Contributor

mmatera commented Feb 6, 2021

Would you please pull out the PossibleZeroQ and put that in its own PR. We'll merge that soon then

See PR #1147

  • I restore SingleLine.send_messages in method in mathics-scanner.feeder, get the CI passing again.

This PR addresses that. So that change can be reverted. Thanks.
Maybe now the simple way would be to make another PR removing the method in mathics-scanner. On the other hand, wouldn't be a good idea to keep this method by now, for backward compatibility?

I am merging this in now. Feel free to keep commenting on it or improve it though.

Merging in will allow mathicsscript and mathics-django to move closer to a release.

@rocky
Copy link
Member Author

rocky commented Feb 6, 2021

See comment on that PR.

@mmatera
Copy link
Contributor

mmatera commented Feb 6, 2021

regarding send_messages in mathics-scanner, you did this to avoid introduce (implicit) references to mathics in mathics-scanner?

@rocky
Copy link
Member Author

rocky commented Feb 6, 2021

Right - this was something that @GarkGarcia originally noticed.

My goal for this weekend though is to get out a release of whatever it is we can. If I had to do again I probably would have totally left in with the Mathics dependencies (as you report doing) and fixed up in a later release.

However since I went down this path, best not to go backwards, but instead go forward it in the places that use mathics scanner and not deal with backwards compatibilty of something that was never released.

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.

2 participants