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

Removed deprecated stuff in Version3_1 branch #126

Merged
merged 14 commits into from
Mar 17, 2024

Conversation

rapoliveira
Copy link
Contributor

@rapoliveira rapoliveira commented Feb 23, 2024

I finished removing all the deprecated functions, arguments and values in Version3_1 branch. Adaptations in the code and in unit tests were necessary in a few cases.

The unit tests keep failing in the same 24 tests as before, mainly in VBBL and PSBL functions (issue #116).

@rapoliveira rapoliveira changed the title Removed deprecated stuff in Version3_1 branch, issue #116 Removed deprecated stuff in Version3_1 branch Feb 23, 2024
@rpoleski
Copy link
Owner

Looks good to me.
@jenniferyee Can we merge it to Version3_1 branch?

@jenniferyee
Copy link
Collaborator

What kind of checks do we have that all of the deprecated code is removed? For example, in Event, we are removing _apply_fit_blending(). Have we verified that there is no longer any code that calls this? I imagine that if we missed removing some instance of fit_blending, then the error the user would see is that _apply_fit_blending() does not exist, which is not very helpful.

Otherwise, fine.

@rpoleski
Copy link
Owner

rpoleski commented Mar 8, 2024

We have tests that pass. They also call ~75% of the code and at some point I've verified that the code not tested is almost exclusively plotting and test of stupid input values. This brings testing of plotting - @rapoliveira can you please run all the examples and check that none of them produces errors? Then we can merge.

@rapoliveira
Copy link
Contributor Author

My last commit updates the few calls to deprecated functions in the examples.

Examples 01, 08, 13, 15, 16, 19, 20, 21 and 23 produce the same error below, which is not related with my changes.
@jenniferyee, could you check it please?

  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/model.py", line 178, in plot_magnification
    magnification = self.get_magnification(
                    ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/model.py", line 1162, in get_magnification
    magnification = self._get_magnification(
                    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/model.py", line 1188, in _get_magnification
    magnification = self._magnification_1_source(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/model.py", line 1250, in _magnification_1_source
    return magnification_curve.get_magnification()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/magnificationcurve.py", line 160, in get_magnification
    magnification = self.get_binary_lens_magnification()
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/magnificationcurve.py", line 524, in get_binary_lens_magnification
    self._magnification_objects[method].get_magnification()
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/binarylens.py", line 1311, in get_magnification
    self.source_x, self.source_y, self.trajectory.parameters.rho,
    ^^^^^^^^^^^^^
  File "/Users/raphael/anaconda3/lib/python3.11/site-packages/MulensModel/binarylens.py", line 724, in source_x
    self._source_x = float(self.trajectory.x)
                     ^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: only length-1 arrays can be converted to Python scalars

@rpoleski
Copy link
Owner

rpoleski commented Mar 8, 2024

Also note that changing to

self._source_x = self.trajectory.x.copy()

produces a different error:

  File "/home/data/caustic/rpoleski/MulensModel/source/MulensModel/magnificationcurve.py", line 524, in get_binary_lens_magnification
    self._magnification_objects[method].get_magnification()
  File "/home/data/caustic/rpoleski/MulensModel/source/MulensModel/binarylens.py", line 692, in get_magnification
    raise NotImplementedError(
NotImplementedError: This is a placeholder class. Use the child classes.

This error repeats for many examples, so there is probably a single error that causes them. @jenniferyee can you solve it?

@rpoleski rpoleski merged commit ff8e9d7 into rpoleski:Version3_1 Mar 17, 2024
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