Skip to content

Fix a number of small bugs#1343

Merged
rocky merged 3 commits intomasterfrom
Get-relative-file
May 2, 2021
Merged

Fix a number of small bugs#1343
rocky merged 3 commits intomasterfrom
Get-relative-file

Conversation

@rocky
Copy link
Copy Markdown
Member

@rocky rocky commented May 2, 2021

  • Synchronize PATH_VAR to $Path at least before mathics_open() is called.
  • Add to_python option string_quotes: boolean. If Falsedon't add extra surrounding quotes.
  • Remove extra space in list in Main Loop doc

Partially addresses #1329

@rocky rocky changed the base branch from master to file-module May 2, 2021 00:28
@rocky rocky requested a review from mmatera May 2, 2021 00:29
@rocky rocky force-pushed the Get-relative-file branch 2 times, most recently from 1eae070 to 2f41b4d Compare May 2, 2021 00:37
* Synchronize PATH_VAR when $Path at least before mathics_open is
called.
* Add to_python option string_quotes: boolean. If False don't add
extra surrounding quotes
* Remove extra space in list in Mainloop doc
@rocky rocky force-pushed the Get-relative-file branch from 2f41b4d to be7bc30 Compare May 2, 2021 00:45
@rocky rocky changed the base branch from file-module to master May 2, 2021 00:47

def to_python(self, *args, **kwargs) -> str:
return '"%s"' % self.value # add quotes to distinguish from Symbols
if kwargs.get("string_quotes", True):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just in case, I shouldn't be useful to have string_quotes with True as default value, for backward compatibility?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't understand. If you haven't set "string_quotes", it defaults to True. Almost all of the code currently is this way: no parameter is set.

If you pass this parameter, then you set it accordingly.

I had thought about a better name, and at some in the past called it something else, but I can't remember the name.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are right! Discard my comment, please.

Copy link
Copy Markdown
Contributor

@mmatera mmatera left a comment

Choose a reason for hiding this comment

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

Apart from the small comment, LGTM

@rocky rocky merged commit 8088091 into master May 2, 2021
@rocky rocky deleted the Get-relative-file branch May 2, 2021 19:41
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