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

Another round of improving the mathics.core.definitions module. #1281

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

mmatera
Copy link
Contributor

@mmatera mmatera commented Jan 10, 2025

  • Simplify arguments in Definition.__init__ method.
  • move autoload_files to mathics.session.
  • move get_file_time to mathics.eval.files_io.files
  • Simplify the way in which the different sets of rules are accessed. Now the parameter
    pos of {get|set}_values include the "values" in the position key. For example, pos="up" now is pos="upvalues". This removes the need to rebuild the key string each time a list of rules is requested.

* Simplify arguments in definition initialization.
* move autoload_files to mathics.session.
* move get_file_time to mathics.eval.files_io.files
* Simplify the way in which the different sets of rules are accessed.
CHANGES.rst Outdated
@@ -62,7 +62,9 @@ API incompatibility

* ``Matcher`` now requires an additional ``evaluation`` parameter
* ``Romberg`` removed as an ``NIntegrate[]`` method. It is depcrecated in SciPy and is to be removed by SciPy 1.15.
* `Definitions.get_ownvalue` now returns a `BaseElement` instead of a `BaseRule` object.
* The signature of the ``Definition.__init__`` now receives a single dict parameter instead of the severa `*values` parameters.
Copy link
Member

@rocky rocky Jan 10, 2025

Choose a reason for hiding this comment

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

severa -> several.

It is probably time to run through codespell.

But note that "inout" and "shbang" are ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did run codespell, and it did not detect it.

@rocky
Copy link
Member

rocky commented Jan 10, 2025

LGTM - a lot of good stuff in here.

I notice we still have a lot of places where we might annotate evaluation with evaluation: Evaluation.

And I saw (but I forget where) where there is this variable, I think it was ironically named "types" which had a changing type value which mypy notices.

But there's always more to do.

@mmatera mmatera merged commit 3d63cf6 into master Jan 10, 2025
14 checks passed
@mmatera mmatera deleted the even_more_on_definitions branch January 10, 2025 12: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.

2 participants