Skip to content

Remove IO dependency#34

Open
limakzi wants to merge 2 commits into
masterfrom
feature/remove-io-external
Open

Remove IO dependency#34
limakzi wants to merge 2 commits into
masterfrom
feature/remove-io-external

Conversation

@limakzi

@limakzi limakzi commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Closes #33. Builds are fixed in #35.

@limakzi limakzi self-assigned this Jul 14, 2026
Copilot AI review requested due to automatic review settings July 14, 2026 11:45

Copilot AI 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.

Pull request overview

Removes the remaining external/network I/O dependency from UnitLib by dropping the IO package requirement and deleting the code path that attempted to fetch order-243 data remotely, aligning behavior with the bundled library data.

Changes:

  • Removed IO from NeededOtherPackages in PackageInfo.g.
  • Deleted the SingleHTTPRequest-based remote fetch path for n=243 and adjusted the boundary condition to treat 243 as bundled.
  • Updated the manual to state that order-243 data is included in the distribution.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
PackageInfo.g Removes the IO package dependency from declared requirements.
lib/unitlib.g Removes remote download logic for n=243 and adjusts the availability boundary condition.
doc/intro.xml Updates installation/library wording to reflect bundled data up to and including 243.
Comments suppressed due to low confidence (1)

lib/unitlib.g:23

  • This PR changes the boundary behavior for n=243 (removes the remote fetch path and treats 243 as bundled), but the test suite does not exercise PcNormalizedUnitGroupSmallGroup(243, ...) at all. Add at least one test case that loads an order-243 entry to prevent regressions and to ensure no network access is required.
if n > 243 then
  Print( "WARNING : the library of V(KG) for groups of order ", n, 
         " is not available yet !!! \n", 
	 "You can use only groups from the unitlib/userdata directory \n",
	 "in case if you already computed their descriptions \n",

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread doc/intro.xml Outdated
Comment thread lib/unitlib.g
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.32%. Comparing base (c34e0e7) to head (3713218).

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
+ Coverage   35.31%   36.32%   +1.00%     
==========================================
  Files           3        3              
  Lines         235      223      -12     
==========================================
- Hits           83       81       -2     
+ Misses        152      142      -10     
Files with missing lines Coverage Δ
lib/unitlib.g 91.02% <100.00%> (+9.91%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@limakzi limakzi changed the title fix: Remove IO dependency Remove IO dependency Jul 14, 2026
@limakzi
limakzi force-pushed the feature/remove-io-external branch from c5d7ecb to 56645b5 Compare July 16, 2026 08:52

@olexandr-konovalov olexandr-konovalov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good, but need to resolve merge conflict first

Comment thread doc/intro.xml Outdated
Comment thread doc/intro.xml
<P/>
The current version of &UnitLib; provides the library of
normalized unit groups <M>V(KG)</M> for all <M>p</M>-groups
of order up to 243.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's write "of orders up to and including 243" here and remove the 1st paragraph of "Install" at all!

Comment thread doc/intro.xml
are included in the distribution. The data for order 243 is available as
an optional download.
The libraries of normalized unit groups of groups of orders up to and including 243 are
part of the distribution.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's write "of orders up to and including 243" above and remove this paragraph of "Install" at all!

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.

Remove attempt to download external data

3 participants