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

Utilize set_angles_from_list, set_angles_from, set_qs_from_range methods later if requested #198

Closed
bobleesj opened this issue Dec 6, 2024 · 4 comments

Comments

@bobleesj
Copy link
Contributor

bobleesj commented Dec 6, 2024

Problem

Our DiffractionObject contains the following methods that are not currently being used:

set_angles_from_list, set_angles_from, set_qs_from_range:

  def set_angles_from_list(self, angles_list):
        self.angles = angles_list
        self.n_steps = len(angles_list) - 1.0
        self.begin_angle = self.angles[0]
        self.end_angle = self.angles[-1]

    def set_qs_from_range(self, begin_q, end_q, step_size=None, n_steps=None):
        """
        create an array of linear spaced Q-values

        Parameters
        ----------
        begin_q float
          the beginning angle
        end_q float
          the ending angle
        step_size float
          the size of the step between points.  Only specify step_size or n_steps, not both
        n_steps integer
          the number of steps.  Odd numbers are preferred. Only specify step_size or n_steps, not both

        Returns
        -------
        Sets self.qs
        self.qs array of floats
          the q values in the independent array

        """
        self.qs = self._set_array_from_range(begin_q, end_q, step_size=step_size, n_steps=n_steps)

    def set_angles_from_range(self, begin_angle, end_angle, step_size=None, n_steps=None):
        """
        create an array of linear spaced angle-values

        Parameters
        ----------
        begin_angle float
          the beginning angle
        end_angle float
          the ending angle
        step_size float
          the size of the step between points.  Only specify step_size or n_steps, not both
        n_steps integer
          the number of steps.  Odd numbers are preferred. Only specify step_size or n_steps, not both

        Returns
        -------
        Sets self.angles
        self.angles array of floats
          the q values in the independent array

        """
        self.angles = self._set_array_from_range(begin_angle, end_angle, step_size=step_size, n_steps=n_steps)

    def _set_array_from_range(self, begin, end, step_size=None, n_steps=None):
        if step_size is not None and n_steps is not None:
            print(
                "WARNING: both step_size and n_steps have been given.  n_steps will be used and step_size will be "
                "reset."
            )
            array = np.linspace(begin, end, n_steps)
        elif step_size is not None:
            array = np.arange(begin, end, step_size)
        elif n_steps is not None:
            array = np.linspace(begin, end, n_steps)
        return array

The above methods could be used for the following UCs in mind written by @sbillinge below:


  1. crystallographer wants to generate an x-array on a grid
  2. crystallogrpher (cr) inputs, start, stop, step, xtype
  3. DO creates xtype-array on this grid, and populates the other x-grids automatically.

I am not sure how this will be used tbh because there needs to be a yarray and this needs a matching xarray, so maybe we don't need this. But if there is already a yarray and the person wants everything on a different grid, a UC would be

  1. cr instantiates a DO with and xarray and a yarray and an xtype
  2. cr wants it on a different xarray
  3. cr supplies start, stop, step xtype
  4. DO generates new xarrays on this grid
  5. DO interpolates the yarray onto this grid and replaces the old yarray with this one

Again, I wonder actually how useful this would be. Maybe it is better to put it into an issue and leave it in case someone asks for it. The same would go for other ways to generate arrays.

Originally posted by @sbillinge in #188 (comment)


Proposed solution

As written above, it would be better to make an issue here and address the above needs once User asks for this specific feature.

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 12, 2024

Update:

... I think we are going to kill for making arrays from ranges. I couldn't come up with a good usecase for them because I think the stay had to be set by the y-array so I couldn't think of a time I would actually use that

by @sbillinge - #53 (comment)

@sbillinge
Copy link
Contributor

I'm closing this. I think it is just distracting keeping it around. Sometimes we want to keep around good ideas for later, but this seems like an idea that (a) isn't good (it was my idea!) (b) if it is useful, it will occur to us later because we need it. Therefore I am closing this.

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 14, 2024

@sbillinge Can I remove these util functions from the package? Since @yucongalicechen has started writing docs and docstrings, thought it would be best to keep it as portable as possible. But, I have copied and pasted those test functions explicitly above.

@sbillinge
Copy link
Contributor

yes, remove all trace of these and any tests.

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

No branches or pull requests

2 participants