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

Edge align Function created #943

Merged
merged 86 commits into from
Apr 12, 2024
Merged

Edge align Function created #943

merged 86 commits into from
Apr 12, 2024

Conversation

MDecarabas
Copy link
Collaborator

  • Created a function that allows for aligning beam with edge

@prjemian
Copy link
Contributor

Note that CI fails on the ruff check:

   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 8.6/8.6 MB 87.0 MB/s eta 0:00:00
Installing collected packages: ruff
Successfully installed ruff-0.3.3
Run ruff . 
warning: `ruff <path>` is deprecated. Use `ruff check <path>` instead.
apstools/plans/alignment.py:21:19: F401 [*] `scipy.stats` imported but unused
apstools/plans/alignment.py:33:8: F401 [*] `warnings` imported but unused
apstools/plans/alignment.py:462:9: F811 Redefinition of unused `stats` from line 21
apstools/plans/alignment.py:470:21: F811 Redefinition of unused `stats` from line 21
apstools/plans/alignment.py:482:18: F811 Redefinition of unused `stats` from line 21
Found 5 errors.
[*] 2 fixable with the `--fix` option.
Error: Process completed with exit code 1.```

@prjemian prjemian added this to the 1.6.19 milestone Mar 20, 2024
@prjemian
Copy link
Contributor

prjemian commented Apr 5, 2024

Will we able to get this resolved and merged in the next week? I'd like to publish the 1.6.19 release next Friday (April 12). Or, could move it to next milestone.

@MDecarabas
Copy link
Collaborator Author

Will we able to get this resolved and merged in the next week? I'd like to publish the 1.6.19 release next Friday (April 12). Or, could move it to next milestone.

We can definitely make that happen.

@prjemian
Copy link
Contributor

@MDecarabas I'm looking for changes that consider the covariance matrix from the fit as a means to validate if the desired feature has been found.

@prjemian
Copy link
Contributor

Still some merge conflicts to resolve.

@prjemian
Copy link
Contributor

Drop in code coverage is understandable. Unit testing would improve that score. (Good for the future.)

CHANGES.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@prjemian prjemian left a comment

Choose a reason for hiding this comment

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

LGTM

@prjemian prjemian self-requested a review April 12, 2024 19:51
@prjemian
Copy link
Contributor

Thought I saw merge conflicts, now there are none, waiting on the tests to run...

@MDecarabas
Copy link
Collaborator Author

Thought I saw merge conflicts, now there are none, waiting on the tests to run...

Yep, there were. But I fixed them right when I saw them

@MDecarabas MDecarabas merged commit af11141 into main Apr 12, 2024
13 checks passed
@MDecarabas MDecarabas deleted the edge_align branch April 12, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can lineup2() identify a leading or following edge?
3 participants