define checkspaces and findfirstblock_sector

[ogauthe]

This PR moves utility functions from FusionTensors.jl to GradedArrays.jl where they belong.

I realized defining BlockArrays.findblock was ambiguous, so I think we need a different name. Here I propose findfirstblock_sector, I would gladly change for better.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.49%. Comparing base (b971b51) to head (5705586).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
+ Coverage   94.40%   94.49%   +0.08%     
==========================================
  Files          20       20              
  Lines         787      799      +12     
==========================================
+ Hits          743      755      +12     
  Misses         44       44              

Flag	Coverage Δ
docs	0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mtfishman]

I realized defining BlockArrays.findblock was ambiguous, so I think we need a different name. Here I propose findfirstblock_sector, I would gladly change for better.

Do you say that BlockArrays.findblock is ambiguous because sectors can be repeated multiple times? If so, I agree findfirstblock is a better name. What about:

function findallblocks(r::AbstractGradedUnitRange, s::AbstractSector)
  return Block.(findall(==(s), sectors(g)))
end

function findfirstblock(g::AbstractGradedUnitRange, s::AbstractSector)
  i = findfirst(==(s), sectors(g))
  isnothing(i) && return nothing
  return Block(i)
end

[ogauthe]

Do you say that BlockArrays.findblock is ambiguous because sectors can be repeated multiple times?

No, it is not about name but about ambiguity error. If I define

BlockArrays.findblock(g::AbstractGradedUnitRange, s) = findfirst(==(s), sectors(g))

g = gradedrange([1=>2])
findblock(g, 1)  # error

We can either specialize BlockArrays.findblock(g::AbstractGradedUnitRange, s::AbstractSector) - but this is less generic - or use another name.

I am fine with defining 2 functions, although I do not really have a use case for findallblocks

[mtfishman]

We can either specialize BlockArrays.findblock(g::AbstractGradedUnitRange, s::AbstractSector) - but this is less generic - or use another name.

Do we have a use case where the sector isn't an AbstractSector? If not I would pick a generic name for the function and then constrain the type. However, I don't think we should call it findblock because that is ambiguous about which block it is finding if more than one block has the same sector. So I would define it as findfirstblock(g::AbstractGradedUnitRange, s::AbstractSector). If we have cases where a sector isn't an AbstractSector we could define findfirstblock_sector later, or at that point we could define a more generic function findfirstblock(f, g::AbstractGradedUnitRange) where f is a function that returns true based on a property of the block.

EDIT: I see in the tests where you're using findfirstblock_sector. Let's define findfirstblock(g::AbstractGradedUnitRange, s::AbstractSector) = findfirstblock_sector(g::AbstractGradedUnitRange, s) and then the implementation can be in findfirstblock_sector.

I am fine with defining 2 functions, although I do not really have a use case for findallblocks

We can hold off on it for now and keep it in mind, partially I'm trying to anticipate other similar functions we might want so we choose a good interface.