Remove IO dependency#34
Conversation
There was a problem hiding this comment.
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
IOfromNeededOtherPackagesinPackageInfo.g. - Deleted the
SingleHTTPRequest-based remote fetch path forn=243and 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.
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
c5d7ecb to
56645b5
Compare
olexandr-konovalov
left a comment
There was a problem hiding this comment.
Sounds good, but need to resolve merge conflict first
| <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. |
There was a problem hiding this comment.
Let's write "of orders up to and including 243" here and remove the 1st paragraph of "Install" at all!
| 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. |
There was a problem hiding this comment.
Let's write "of orders up to and including 243" above and remove this paragraph of "Install" at all!
Closes #33. Builds are fixed in #35.