Skip to content

feat(#4952): moved acos from real.eo to a separate object with tests#4961

Merged
yegor256 merged 1 commit into
objectionary:masterfrom
eshabakhov:#4952
Apr 1, 2026
Merged

feat(#4952): moved acos from real.eo to a separate object with tests#4961
yegor256 merged 1 commit into
objectionary:masterfrom
eshabakhov:#4952

Conversation

@eshabakhov

@eshabakhov eshabakhov commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR solves #4952

Changes

  • removed acos from real.eo
  • added acos in acos.eo with tests
  • fixed EOreal$EOacos.java to EOacos.java

Summary by CodeRabbit

  • New Features

    • Added a standalone arc-cosine (acos) operation accessible from the math utilities.
    • Included executable tests for exact values, floating-point accuracy, and NaN/infinity edge cases.
  • Bug Fixes / API Changes

    • acos removed from the previous real-number object and now provided via the new module; related public aliases adjusted.

@coderabbitai

coderabbitai Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 48cb4398-3eff-4076-9e5c-53dfb6aad7d9

📥 Commits

Reviewing files that changed from the base of the PR and between 52219b5 and 25f49f6.

📒 Files selected for processing (3)
  • eo-runtime/src/main/eo/ms/acos.eo
  • eo-runtime/src/main/eo/ms/real.eo
  • eo-runtime/src/main/java/org/eolang/EOms/EOacos.java
💤 Files with no reviewable changes (1)
  • eo-runtime/src/main/eo/ms/real.eo
🚧 Files skipped from review as they are similar to previous changes (2)
  • eo-runtime/src/main/eo/ms/acos.eo
  • eo-runtime/src/main/java/org/eolang/EOms/EOacos.java

📝 Walkthrough

Walkthrough

This PR extracts the arc-cosine operation into a new ms.acos EO module with tests, removes acos and the ms.pi alias from ms.real, and renames/refactors the Java implementation class to EOacos with a dedicated "num" input slot.

Changes

Cohort / File(s) Summary
New ms.acos Module
eo-runtime/src/main/eo/ms/acos.eo
Adds new ms.acos EO module exporting [num] > acos ? ..., with executable tests for exact values (±1, 0), accuracy checks (±0.6 within 1e-6), NaN propagation, and infinity/out-of-range handling.
ms.real Cleanup
eo-runtime/src/main/eo/ms/real.eo
Removes [] > acos ? from real and deletes related acos tests; removes alias ms.pi from module exports.
Java Class Refactoring
eo-runtime/src/main/java/org/eolang/EOms/EOacos.java
Renames class from EOreal$EOacosEOacos, changes @XmirObject(oname="real.acos")oname="acos", adds public no-arg constructor registering "num" slot, and updates lambda() to use this.take("num").

Sequence Diagram(s)

(Skipped — changes are module extraction and class refactor without new multi-component control flow.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped from real to a brand-new place,

acos stands proud with tests in its space.
Java renamed and inputs well-defined,
Numbers and NaN handled, all aligned.
A tiny rabbit cheers this tidy refactor race.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: moving the acos operation from the real.eo module to a separate acos.eo module with comprehensive test coverage, which matches the file alterations and PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@eshabakhov eshabakhov changed the title feat(#4952): move acos from real.eo to a separate object with tests feat(#4952): moved acos from real.eo to a separate object with tests Mar 27, 2026

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eo-runtime/src/main/eo/ms/acos.eo`:
- Around line 13-14: The ms.real.acos stub left behind should either be removed
or turned into an explicit forwarder to the new ms.acos implementation: locate
the public object declaration named ms.real.acos (the lone "[] > acos ?" stub)
and either delete that stub entirely or change it to forward calls to ms.acos
(preserving the original signature so callers still resolve), ensuring the Java
atom `@XmirObject`(oname = "acos") provided by ms.acos is the effective
implementation.

In `@eo-runtime/src/main/java/org/eolang/EOms/EOacos.java`:
- Around line 12-17: The code uses an unqualified nested class ToPhi which will
not compile; import org.eolang.Data (or reference Data) and replace the new
ToPhi(...) usage with new Data.ToPhi(...). Update the imports to include
org.eolang.Data and change the constructor call in EOacos (the spot creating
ToPhi) to use the nested class qualifier Data.ToPhi so the compiler can resolve
the nested class.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 25dfade9-00ec-4066-8b3d-9e280090f52d

📥 Commits

Reviewing files that changed from the base of the PR and between c8dfb01 and 52219b5.

📒 Files selected for processing (3)
  • eo-runtime/src/main/eo/ms/acos.eo
  • eo-runtime/src/main/eo/ms/real.eo
  • eo-runtime/src/main/java/org/eolang/EOms/EOacos.java
💤 Files with no reviewable changes (1)
  • eo-runtime/src/main/eo/ms/real.eo

Comment thread eo-runtime/src/main/eo/ms/acos.eo
Comment thread eo-runtime/src/main/java/org/eolang/EOms/EOacos.java
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Performance Analysis

All benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information.

Click to see the detailed report
Test Base Score PR Score Change % Change Unit Mode
benchmarks.XmirBench.xmirToEO 180.824 170.680 -10.145 -5.61% ms/op Average Time

✅ Performance gain: benchmarks.XmirBench.xmirToEO is faster by 10.145 ms/op (5.61%)

@eshabakhov

Copy link
Copy Markdown
Contributor Author

@yegor256 could you check?

@yegor256 yegor256 merged commit c9ed505 into objectionary:master Apr 1, 2026
25 checks passed
@yegor256

yegor256 commented Apr 1, 2026

Copy link
Copy Markdown
Member

@eshabakhov thanks!

@0crat

0crat commented Apr 1, 2026

Copy link
Copy Markdown

@eshabakhov Thanks for your valuable contribution! 🎉 You've earned +12 points (+16 base, -4 for code size optimization). Your running score is now +80 - great progress! Please keep contributions focused under 100 hits-of-code for maximum points, and don't forget to check your Zerocracy account for updates.

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